Thread overview
[Issue 5157] New: thread_joinall() looks like it doesn't actually join all
Nov 03, 2010
Jonathan M Davis
Nov 03, 2010
Jonathan M Davis
November 03, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5157

           Summary: thread_joinall() looks like it doesn't actually join
                    all
           Product: D
           Version: unspecified
          Platform: Other
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: druntime
        AssignedTo: sean@invisibleduck.org
        ReportedBy: jmdavisProg@gmx.com


--- Comment #0 from Jonathan M Davis <jmdavisProg@gmx.com> 2010-11-02 20:46:51 PDT ---
In core.threads, there is a function called thread_joinall() which supposedly joins all non-daemon threads. At present, it's implementation is

extern (C) void thread_joinAll()
{

    while( true )
    {
        Thread nonDaemon = null;

        foreach( t; Thread )
        {
            if( !t.isDaemon )
            {
                nonDaemon = t;
                break;
            }
        }
        if( nonDaemon is null )
            return;
        nonDaemon.join();
    }
}


Notice that the last if statement _returns_ if the thread is a daemon thread. That means that the function joins every thread until it finds a single daemon thread, so it's _not_ going to join all non-daemon threads unless there are no daemon threads or all of the daemon threads are at the end of the range generated by opApply(). So, if I understand correctly what the function is supposed to be doing, it should be something more like this:


extern (C) void thread_joinAll()
{
    foreach( t; Thread )
    {
        if( !t.isDaemon )
            t.join();
    }
}


This still might have issues though if the list of running threads changes while thread_joinAll() is running. I think that opApply() will iterate over the list of threads as they exist when opApply() is called, but if any threads are joined while opApply() is running (likely because joining a thread causes another thread to join in addition to the one join() was called on), then you could get a double-join happening, which would currently result in an exception being thrown. So, there may also need to be a way to verify that a thread hasn't been joined before attempting to join it. And of course, some locking primitives may be necessary to make sure that there aren't any race conditions in doing so.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
November 03, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5157


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |schveiguy@yahoo.com
         Resolution|                            |INVALID


--- Comment #1 from Steven Schveighoffer <schveiguy@yahoo.com> 2010-11-03 07:55:06 PDT ---
The original code looks correct to me.  Rewritten in pseudocode:

while(true)
{
   nonDaemon = null;
   assign nonDaemon to first non-daemon thread in the list of threads;
   if(nonDaemon not found)
     // this means there are no non-daemon threads left
     return;
   nonDaemon.join();
}

The reason it is done this way instead of the way you suggest is because the foreach is synchronized on a lock (to prevent threads coming/going while iterating).  So you don't want to call join while inside the foreach loop.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
November 03, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5157



--- Comment #2 from Jonathan M Davis <jmdavisProg@gmx.com> 2010-11-03 10:24:45 PDT ---
You're right. I guess that I was just looking at it too closely or something. But on another inspection, it does seem to be correct. It will loop through all of them until it finds a non-daemon thread or it loops through all of them, and only then will it end up returning. So, this bug isn't a bug.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------