Threading problem.

[muscha]

Hi All,

I'm having a bit of problem with threading, when I first call Start() on my
code the thread work, I call Stop() and it stopped, but when I call Start()
again my thread is not started. Although in the debugger I can see that it
went to the Start() method and creating the thread again.

Can anyone help me? This is the code:

<snip>

public static MySingleton GetInstance() {
if (instance == null) {
instance = new MySingleton();
}
return instance;
}

public void Start() {
lock (this) {
if (!started){
started = true;
thread = new Thread(new ThreadStart(Run));
thread.Start();
}
}
}

public void Stop() {
stopped = true;
}

private void Run() {
while (!stopped) {
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
thread.Join();
started = false;
}

</snip>
Thanks,

/m

[Dan Cimpoiesu]

Try thread.Abort() on the Stop method

Hope this helps
Dan Cimpoiesu

"muscha" <mu****@no.spam.net> wrote in message
news:eh*************@tk2msftngp13.phx.gbl...

Hi All,

I'm having a bit of problem with threading, when I first call Start() on my code the thread work, I call Stop() and it stopped, but when I call Start() again my thread is not started. Although in the debugger I can see that it
went to the Start() method and creating the thread again.

Can anyone help me? This is the code:

<snip>

public static MySingleton GetInstance() {
if (instance == null) {
instance = new MySingleton();
}
return instance;
}

public void Start() {
lock (this) {
if (!started){
started = true;
thread = new Thread(new ThreadStart(Run));
thread.Start();
}
}
}

public void Stop() {
stopped = true;
}

private void Run() {
while (!stopped) {
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
thread.Join();
started = false;
}

</snip>
Thanks,

/m

[Jon Skeet]

muscha <mu****@no.spam.net> wrote:

I'm having a bit of problem with threading, when I first call Start() on my
code the thread work, I call Stop() and it stopped, but when I call Start()
again my thread is not started. Although in the debugger I can see that it
went to the Start() method and creating the thread again.

Your code is slightly odd - I don't see why you're trying to call
thread.Join() within Run, when the thread it's running in should be the
thread it's trying to join anyway!

Note also that your singleton implementation isn't threadsafe - see
http://www.pobox.com/~skeet/csharp/singleton.html

Finally, the way you're testing the "stopped" flag is dodgy - because
you haven't got any synchronization there, there's no guarantee that
other threads will see new values.

If you could post a short but *complete* example which demonstrates the
problem, that would help.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet
If replying to the group, please do not mail me too

[José Joye]

Change the 2 methods below and it should work:

public void Stop()
{
lock (this)
{
stopped = true;
thread.Join();
started = false;
stopped = false;
}
}

private void Run()
{
while (!stopped)
{
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
}

José
"muscha" <mu****@no.spam.net> wrote in message
news:eh*************@tk2msftngp13.phx.gbl...

Hi All,

I'm having a bit of problem with threading, when I first call Start() on my code the thread work, I call Stop() and it stopped, but when I call Start() again my thread is not started. Although in the debugger I can see that it
went to the Start() method and creating the thread again.

Can anyone help me? This is the code:

<snip>

public static MySingleton GetInstance() {
if (instance == null) {
instance = new MySingleton();
}
return instance;
}

public void Start() {
lock (this) {
if (!started){
started = true;
thread = new Thread(new ThreadStart(Run));
thread.Start();
}
}
}

public void Stop() {
stopped = true;
}

private void Run() {
while (!stopped) {
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
thread.Join();
started = false;
}

</snip>
Thanks,

/m

[muscha]

Jon and all,

Note also that your singleton implementation isn't threadsafe - see
http://www.pobox.com/~skeet/csharp/singleton.html
Thanks for that.
If you could post a short but *complete* example which demonstrates the
problem, that would help.

Here is my class, I followed people's suggestion and it seems working now,
however is it thread safe?
using System;
using System.Threading;

namespace Scheduler
{
public class MySingleton
{
private static MySingleton instance;
private static Object padlock = new Object();
private Thread thread;
private bool stopped = false;
private bool started = false;

/// <summary>
/// Private constructor so this thread can't be instantiated.
/// </summary>
private MySingleton()
{
}

~MySingleton()
{
Stop();
}

public static MySingletonGetInstance()
{
if (instance == null)
{
lock (padlock)
{
if (instance == null)
{
instance = new MySingleton();
}
}
}
return instance;
}

public void Start()
{
lock (this)
{
if (!started)
{
started = true;
thread = new Thread(new ThreadStart(this.Run));
thread.Start();
}
}
}

public void Stop()
{
lock (this)
{
stopped = true;
thread.Join();
started = false;
stopped = false;
}
}

private void Run()
{
while (!stopped)
{
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
}
}
}

Thanks,

/m

[muscha]

Jon and all,

Note also that your singleton implementation isn't threadsafe - see
http://www.pobox.com/~skeet/csharp/singleton.html
Thanks for that.
If you could post a short but *complete* example which demonstrates the
problem, that would help.

Here is my class, I followed people's suggestion and it seems working now,
however is it thread safe?
using System;
using System.Threading;

namespace Scheduler
{
public class MySingleton
{
private static MySingleton instance;
private static Object padlock = new Object();
private Thread thread;
private bool stopped = false;
private bool started = false;

/// <summary>
/// Private constructor so this thread can't be instantiated.
/// </summary>
private MySingleton()
{
}

~MySingleton()
{
Stop();
}

public static MySingletonGetInstance()
{
if (instance == null)
{
lock (padlock)
{
if (instance == null)
{
instance = new MySingleton();
}
}
}
return instance;
}

public void Start()
{
lock (this)
{
if (!started)
{
started = true;
thread = new Thread(new ThreadStart(this.Run));
thread.Start();
}
}
}

public void Stop()
{
lock (this)
{
stopped = true;
thread.Join();
started = false;
stopped = false;
}
}

private void Run()
{
while (!stopped)
{
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
}
}
}

Thanks,

/m

[Doug Forster]

Hi Muscha,

Looks to be thread safe after a quick glance anyway. It it were my code I
would first check that the thread is running in Stop to guard against the
possibility that Start was never called.

Cheers

Doug Forster

"muscha" <mu****@no.spam.net> wrote in message
news:uo*************@TK2MSFTNGP11.phx.gbl...

Jon and all,

Note also that your singleton implementation isn't threadsafe - see
http://www.pobox.com/~skeet/csharp/singleton.html

Thanks for that.

If you could post a short but *complete* example which demonstrates the
problem, that would help.

Here is my class, I followed people's suggestion and it seems working now,
however is it thread safe?
using System;
using System.Threading;

namespace Scheduler
{
public class MySingleton
{
private static MySingleton instance;
private static Object padlock = new Object();
private Thread thread;
private bool stopped = false;
private bool started = false;

/// <summary>
/// Private constructor so this thread can't be instantiated.
/// </summary>
private MySingleton()
{
}

~MySingleton()
{
Stop();
}

public static MySingletonGetInstance()
{
if (instance == null)
{
lock (padlock)
{
if (instance == null)
{
instance = new MySingleton();
}
}
}
return instance;
}

public void Start()
{
lock (this)
{
if (!started)
{
started = true;
thread = new Thread(new ThreadStart(this.Run));
thread.Start();
}
}
}

public void Stop()
{
lock (this)
{
stopped = true;
thread.Join();
started = false;
stopped = false;
}
}

private void Run()
{
while (!stopped)
{
Console.WriteLine("Running ...");
Thread.Sleep(1000);
}
}
}
}

Thanks,

/m

[Jon Skeet]

muscha <mu****@no.spam.net> wrote:

Jon and all,

Note also that your singleton implementation isn't threadsafe - see
http://www.pobox.com/~skeet/csharp/singleton.html

Thanks for that.

I note that you're using the double-checked locking algorithm though,
which I don't believe to be thread-safe.

If you could post a short but *complete* example which demonstrates the
problem, that would help.

Here is my class, I followed people's suggestion and it seems working now,
however is it thread safe?

Not quite. Aside from the singleton problem, you're not locking on
anything before checking the stopped flag in Run. I would also suggest
not locking on "this" but locking on another reference private to the
class (just create a new object, like padlock but with an instance
variable).

I'm also not keen on the finalizer - do you think you really need it,
particularly? It's only going to be invoked when the AppDomain is
unloaded, and then only "some time after that". I would personaly just
get rid of it.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet
If replying to the group, please do not mail me too

[muscha]

> > Here is my class, I followed people's suggestion and it seems working
now,

however is it thread safe?

Not quite. Aside from the singleton problem, you're not locking on
anything before checking the stopped flag in Run. I would also suggest
not locking on "this" but locking on another reference private to the
class (just create a new object, like padlock but with an instance
variable).

What's the difference from locking a separate object to lock than doing lock
on this? Isn't that by doing lock(this) in the beginning of the method, you
practically synchronize the entire method call?

thanks,

/m

[Jon Skeet]

muscha <mu****@no.spam.net> wrote:

Not quite. Aside from the singleton problem, you're not locking on
anything before checking the stopped flag in Run. I would also suggest
not locking on "this" but locking on another reference private to the
class (just create a new object, like padlock but with an instance
variable).

What's the difference from locking a separate object to lock than doing lock
on this? Isn't that by doing lock(this) in the beginning of the method, you
practically synchronize the entire method call?

If you lock on this, then something else might also lock on it, and you
could end up with a deadlock. If you lock on a reference that only your
instance will ever know about, you have more control.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet
If replying to the group, please do not mail me too

[Jon Skeet]

"Doug Forster" <doug_ZAPTHIS_AT_TONIQ_ZAPTHIS_co.nz> wrote:

Looks to be thread safe after a quick glance anyway.
And that's why you should never trust multithreading code to quick
glances :)
It it were my code I
would first check that the thread is running in Stop to guard against the
possibility that Start was never called.

That would indeed be a good thing to do - I missed that.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet
If replying to the group, please do not mail me too