www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Threads not garbage collected ?

reply Alex <alex.spam sunopti.com> writes:
import core.thread;

class A
{
	Thread mThread;
	bool mStopped;
	
	this()
	{
		mThread = new Thread(&run);
		mThread.start();
	}
	
	void run()
	{
		while (!mStopped)
		{
			//do stuff
		}
	}
	~this()
	{
		mStopped = true;
		mThread.join();
	}
}



void main()
{
	auto a = new A;
	delete a;		//need this or the program hangs
}

In both gdc and dmd I need to use manually delete this object or 
the program is blocked after main. Is by design ?
It seems undesirable, as the thread can be many layers of 
encapsulation down, and they all need manual deletes.
Feb 21 2017
next sibling parent Jerry <hurricane hereiam.com> writes:
Not really by design so much as you can't really guarantee 
destruction because it is garbage collected. You can use a struct 
instead or scoped 
(https://dlang.org/library/std/typecons/scoped.html).
Feb 21 2017
prev sibling next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
On 22/02/2017 6:28 PM, Alex wrote:

 import core.thread;

 class A
 {
     Thread mThread;
     bool mStopped;

     this()
     {
         mThread = new Thread(&run);
         mThread.start();
     }

     void run()
     {
         while (!mStopped)
         {
             //do stuff
         }
     }
     ~this()
     {
         mStopped = true;
         mThread.join();
     }
 }



 void main()
 {
     auto a = new A;
     delete a;        //need this or the program hangs
 }

 In both gdc and dmd I need to use manually delete this object or the
 program is blocked after main. Is by design ?
 It seems undesirable, as the thread can be many layers of encapsulation
 down, and they all need manual deletes.


Well, you never told the program to stop without delete.

The thread that you start with (calls the main function) doesn't 
actually have to stay alive throughout the program running, surprise!

Anyway, if you do want something that will stop try:

import core.thread;
import core.time;
import std.stdio;

class Foo : Thread {
         bool keepRunning;

         this() {
                 keepRunning = true;
                 super(&run);
         }

         private void run() {
                 while(keepRunning) {
                         sleep(1.seconds);
                         writeln("iterate");
                 }
         }
}

void main() {
         Foo foo = new Foo;
         foo.start;

         Thread.sleep(3.seconds);
         foo.keepRunning = false;
}
Feb 21 2017
parent Alex <alexibu.nospam me.com> writes:
On Wednesday, 22 February 2017 at 05:39:50 UTC, rikki cattermole 
wrote:

 On 22/02/2017 6:28 PM, Alex wrote:

 import core.thread;

 class A
 {
     Thread mThread;
     bool mStopped;

     this()
     {
         mThread = new Thread(&run);
         mThread.start();
     }

     void run()
     {
         while (!mStopped)
         {
             //do stuff
         }
     }
     ~this()
     {
         mStopped = true;
         mThread.join();
     }
 }



 void main()
 {
     auto a = new A;
     delete a;        //need this or the program hangs
 }

 In both gdc and dmd I need to use manually delete this object 
 or the
 program is blocked after main. Is by design ?
 It seems undesirable, as the thread can be many layers of 
 encapsulation
 down, and they all need manual deletes.


 Well, you never told the program to stop without delete.

 The thread that you start with (calls the main function) 
 doesn't actually have to stay alive throughout the program 
 running, surprise!

 Anyway, if you do want something that will stop try:

 import core.thread;
 import core.time;
 import std.stdio;

 class Foo : Thread {
         bool keepRunning;

         this() {
                 keepRunning = true;
                 super(&run);
         }

         private void run() {
                 while(keepRunning) {
                         sleep(1.seconds);
                         writeln("iterate");
                 }
         }
 }

 void main() {
         Foo foo = new Foo;
         foo.start;

         Thread.sleep(3.seconds);
         foo.keepRunning = false;
 }


That is interesting, I did not know that the main thread could 
exit without returning control to the calling process.
Maybe this should be the case if we were writing in raw C against 
OS threading calls, but in this case I clearly have a thread 
Object not just an OS Handle, and I think most people would have 
an expectation that the garbage collector looks after deleting 
things in a garbage collected language when all the references 
are gone.

I have also tested throwing an exception in main, and yes the 
program hangs. This is even more of a problem because this 
undesirable behaviour may only be observed under exceptional 
circumstances.
Feb 22 2017
prev sibling next sibling parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Wednesday, February 22, 2017 05:28:17 Alex via Digitalmars-d wrote:

 import core.thread;

 class A
 {
   Thread mThread;
   bool mStopped;

   this()
   {
       mThread = new Thread(&run);
       mThread.start();
   }

   void run()
   {
       while (!mStopped)
       {
           //do stuff
       }
   }
   ~this()
   {
       mStopped = true;
       mThread.join();
   }
 }



 void main()
 {
   auto a = new A;
   delete a;       //need this or the program hangs
 }

 In both gdc and dmd I need to use manually delete this object or
 the program is blocked after main. Is by design ?
 It seems undesirable, as the thread can be many layers of
 encapsulation down, and they all need manual deletes.


There's never a guarantee that an object is going to be collected. The
program is free to not bother to collect any GC-allocated objects when it
exits. Normally, the GC only attempts to collect objects and free memory
when you call new, because that's when it needs more memory.

And because you put the join in a class finalizer, the thread won't be
joined unless the GC happens to decide to collect that class - which it will
never do, because new isn't being called, and the open thread keeps the
program running.

If you want deterministic or guaranteed destruction, you should use a
struct on the stack, not a class. In general, managing resources in a class
finalizer is just begging for them to never be released, and it should
really only be done as a backup for when the resources aren't explicitly
freed by the programmer.

In this particular case, the program would probably exit if you did

void main()
{
    {
        auto a = new A;
    }

    import core.memory;
    GC.collect();
}

but really, relying on a class' finalizer to be called in order to join a
thread is begging for trouble.

- Jonathan M Davis
Feb 21 2017
parent reply David Nadlinger <code klickverbot.at> writes:
On Wednesday, 22 February 2017 at 07:14:27 UTC, Jonathan M Davis 
wrote:

 In this particular case, the program would probably exit if you 
 did

 void main()
 {
     {
         auto a = new A;
     }

     import core.memory;
     GC.collect();
 }


I very much doubt so. A Thread object isn't just a handle or 
thread reference, but also encapsulates some of the 
system/druntime state required to make threading work. Executing 
Thread.~this() while a thread was still running would lead to a 
crash.

(The tricky part is to make sure the Thread object becomes 
eligible for collection once a thread has actually finished.)

  — David
Feb 21 2017
parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Wednesday, February 22, 2017 07:58:45 David Nadlinger via Digitalmars-d 
wrote:

 On Wednesday, 22 February 2017 at 07:14:27 UTC, Jonathan M Davis

 wrote:

 In this particular case, the program would probably exit if you
 did

 void main()
 {

     {

         auto a = new A;

     }

     import core.memory;
     GC.collect();

 }


 I very much doubt so. A Thread object isn't just a handle or
 thread reference, but also encapsulates some of the
 system/druntime state required to make threading work. Executing
 Thread.~this() while a thread was still running would lead to a
 crash.

 (The tricky part is to make sure the Thread object becomes
 eligible for collection once a thread has actually finished.)


Well, the OP's code wrapped the Thread object in another class and joined
the thread in its finalizer, so you would think that the Thread would be
joined before its finalizer was called, but thinking further on it, IIRC,
there's no guarantee that the GC wouldn't try and collect the Thread object
before calling the finalizer (since there are no other references to the
Thread). So, I suppose that it could fail because of that. But if that
doesn't happen, it should work, because the wrapper calls join rather than
simply letting the Thread be destroyed.

In any case, I don't know if manually running a collection like I showed
would work or not, but if not, I wouldn't expect calling join in a finalizer
to work in general.

- Jonathan M Davis
Feb 22 2017
parent David Nadlinger <code klickverbot.at> writes:
On Wednesday, 22 February 2017 at 08:20:41 UTC, Jonathan M Davis 
wrote:

 Well, the OP's code wrapped the Thread object in another class 
 and joined the thread in its finalizer, so you would think that 
 the Thread would be joined before its finalizer was called, but 
 thinking further on it, IIRC, there's no guarantee that the GC 
 wouldn't try and collect the Thread object before calling the 
 finalizer (since there are no other references to the Thread). 
 So, I suppose that it could fail because of that. But if that 
 doesn't happen, it should work, because the wrapper calls join 
 rather than simply letting the Thread be destroyed.


True – I was thinking of the subclassing example also shown above.


 In any case, I don't know if manually running a collection like 
 I showed would work or not, but if not, I wouldn't expect 
 calling join in a finalizer to work in general.


Yes, it wouldn't work in general – mThread isn't guaranteed to be 
around any longer in ~this() for a normal object (although the 
thread lifetime relationship mentioned above complicates the 
analysis here somewhat, and if the thread is never stopped 
somewhere else, it might just as well work by accident).

  — David
Feb 22 2017
prev sibling next sibling parent reply ketmar <ketmar ketmar.no-ip.org> writes:
Alex wrote:


 import core.thread;

 class A
 {
 	Thread mThread;
 	bool mStopped;
 	 this()
 	{
 		mThread = new Thread(&run);
 		mThread.start();
 	}
 	 void run()
 	{
 		while (!mStopped)
 		{
 			//do stuff
 		}
 	}
 	~this()
 	{
 		mStopped = true;
 		mThread.join();
 	}
 }


this code is invalid. when `~this()` is called, your `mThread` *may* 
already be dead -- this is how GC works. you *may* *not* access 
heap-allocated members in `~this()`, this is a bug. it may work 
sometimes, but it isn't guaranteed.
Feb 22 2017
parent reply Alex <alexibu.nospam me.com> writes:
On Wednesday, 22 February 2017 at 08:28:48 UTC, ketmar wrote:

 Alex wrote:


 import core.thread;

 class A
 {
 	Thread mThread;
 	bool mStopped;
 	 this()
 	{
 		mThread = new Thread(&run);
 		mThread.start();
 	}
 	 void run()
 	{
 		while (!mStopped)
 		{
 			//do stuff
 		}
 	}
 	~this()
 	{
 		mStopped = true;
 		mThread.join();
 	}
 }


 this code is invalid. when `~this()` is called, your `mThread` 
 *may* already be dead -- this is how GC works. you *may* *not* 
 access heap-allocated members in `~this()`, this is a bug. it 
 may work sometimes, but it isn't guaranteed.


Agreed, I don't want to have a ~this(), I have added it to 
prevent my program from hanging.
As is often the case with these examples, it is a distillation of 
a much larger project.

The point is that the thread object could be 20 layers of 
encapsulation down in a library.
The thread could also be added 20 layers down long after the main 
function was written and tested.

The thread can then prevent the program from exiting on exception 
or otherwise.
If the garbage collector doesn't kill threads, do I need to break 
all encapsulation to call some sort of finalise or destroy 
function on every object in case it has a thread object in it ?
It would probably be better to have all core.thread.Threads 
registered in the run time so they can be killed when main exits.
Feb 22 2017
next sibling parent ketmar <ketmar ketmar.no-ip.org> writes:
Alex wrote:


 The thread can then prevent the program from exiting on exception or 
 otherwise.
 If the garbage collector doesn't kill threads, do I need to break all 
 encapsulation to call some sort of finalise or destroy function on 
 every object in case it has a thread object in it ?
 It would probably be better to have all core.thread.Threads 
 registered in the run time so they can be killed when main exits.


they are registered. but not exiting while at least one thread is alive 
is deliberate decision.

you may take a look at std.concurrency instead of working with "raw" 
threads. besides providing services like message passing, it does 
terminate all the spawned threads if their owner dies. you may change 
that, of course, this is just a convenient default.

but note that such termination will require calling `receive()` in a 
spawned thread. still, i think that message passing feature of 
std.concurrency is so useful that there is no reason to bypass it and 
invent your own. besides, you may just call `receiveTimeout()` with 
small timeout (not zero, there *was* a bug with zero == infinity) in 
your thread even if you aren't using standard message passing 
mechanics; it is completely harmless.
Feb 22 2017
prev sibling parent Guillaume Piolat <first.last gmail.com> writes:
On Wednesday, 22 February 2017 at 11:29:27 UTC, Alex wrote:

 The point is that the thread object could be 20 layers of 
 encapsulation down in a library.
 The thread could also be added 20 layers down long after the 
 main function was written and tested.


Here is what works for me:
- having a clear owner for things that may own resource 
transitively,
    => an acyclic graph of objects
- use destructors, _but ensure they are called manually not by 
the GC_,
- in each destructor, call .destroy on every members that are 
classes (in the right order if it matters),
- call .destroy on the top-level object to exit

It was insane before I did that.
Feb 23 2017
prev sibling next sibling parent reply Guillaume Piolat <first.last gmail.com> writes:
On Wednesday, 22 February 2017 at 05:28:17 UTC, Alex wrote:

 void main()
 {
 	auto a = new A;
 	delete a;		//need this or the program hangs
 }


https://p0nce.github.io/d-idioms/#The-trouble-with-class-destructors
Feb 22 2017
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Wednesday, 22 February 2017 at 14:51:24 UTC, Guillaume Piolat 
wrote:

 https://p0nce.github.io/d-idioms/#The-trouble-with-class-destructors


eh, I think this is less a destructor issue and just that the 
thread is still running and thus not eligible for collection 
anyway - there's still a reference to it elsewhere.
Feb 22 2017
prev sibling next sibling parent reply Moritz Maxeiner <moritz ucworks.org> writes:
On Wednesday, 22 February 2017 at 05:28:17 UTC, Alex wrote:

 [...]

 In both gdc and dmd I need to use manually delete this object 
 or the program is blocked after main. Is by design ?


Yes, it's documented here[1] (others have already replied on the 
GC subject, so I won't go into that). If you want a thread to be 
forcibly terminated instead of preventing program termination you 
need to set `thread.isDaemon = true`.


 It seems undesirable, as the thread can be many layers of 
 encapsulation down, and they all need manual deletes.


It's highly desirable to not have the program terminate when 
there's still work to be done on some critical non-main thread, 
so the druntime split into daemon vs non-daemon threads makes 
sense.
Whether new threads should be daemonized by default (as opposed 
to preventing program termination by default) is an arbitrary 
decision. In your case, just set `isDaemon = true` for your 
threads and you should be good.

[1] https://dlang.org/phobos/core_thread.html#.Thread.isDaemon
Feb 22 2017
parent Alex <alex.nospam me.com> writes:
On Wednesday, 22 February 2017 at 15:23:59 UTC, Moritz Maxeiner 
wrote:

 It seems undesirable, as the thread can be many layers of 
 encapsulation down, and they all need manual deletes.


 It's highly desirable to not have the program terminate when 
 there's still work to be done on some critical non-main thread, 
 so the druntime split into daemon vs non-daemon threads makes 
 sense.
 Whether new threads should be daemonized by default (as opposed 
 to preventing program termination by default) is an arbitrary 
 decision. In your case, just set `isDaemon = true` for your 
 threads and you should be good.

 [1] https://dlang.org/phobos/core_thread.html#.Thread.isDaemon


Thanks, that is what I need.
Feb 22 2017
prev sibling parent Chris Wright <dhasenan gmail.com> writes:
The thread you created holds a reference to the `class A` object, so that 
object can't be collected.

This is what you probably want. As soon as the object is collected, its 
memory can be reused. But the thread still has a reference to it, so it 
can modify and read that memory. And whoever the GC handed that memory can 
modify and read it. In short, data corruption.
Feb 22 2017