Bug 3054 – multithreading GC problem. And Stdio not multithreading safe
Status
RESOLVED
Resolution
FIXED
Severity
critical
Priority
P2
Component
phobos
Product
D
Version
D2
Platform
Other
OS
Windows
Creation time
2009-06-06T08:08:00Z
Last change time
2014-07-03T00:05:56Z
Assigned to
nobody
Creator
davidl
Comments
Comment #0 by davidl — 2009-06-06T08:08:57Z
import core.thread;
import std.stdio;
class threadid
{
static int threadid;
static int threadnum;
int getId(){threadid++; return threadid; }
}
threadid TID;
void main()
{
TID = new threadid;
while(true)
{
try
{
synchronized(TID) if (TID.threadnum<500)
{
auto stress = (new Thread(
(){
int tid;
synchronized(TID){ tid = TID.getId(); }
scope(exit) synchronized(TID){TID.threadnum--;}
synchronized(TID){TID.threadnum++;}
//writefln("new thread:", tid, TID.threadnum);
void[] buffer;
try
{
buffer.length= 65536;
}
catch(Exception e)
{
writefln("thread:", tid);
writefln(e.msg);
}
}
));
stress.start();
}
//writefln("outside:", TID.threadnum);
}
catch(Exception e)
{
//writefln("error: ", e.msg);
}
}
}
Comment #1 by davidl — 2009-06-14T07:52:45Z
the core.thread gets two problem:
1.
t = new Thread(
(){
while(true){}
}
);
delete t;
The last line should stuck there, because the thread is still running.
A possible solution is add a join in the dtor. It's quite fair your code stuck there if the thread is still running.
Another problem is thread_scanAll.
extern (C) void thread_scanAll( scanAllThreadsFn scan, void* curStackTop = null )
Notice thread_scanAll may happen before the m_tls is set.
so the for loop:
for( Thread t = Thread.sm_tbeg; t; t = t.next )
{
+++ if (t.tlsvalid)
scan( &t.m_tls[0], &t.m_tls[0] + t.m_tls.length );
version( Windows )
{
scan( &t.m_reg[0], &t.m_reg[0] + t.m_reg.length );
}
}
add a tlsvalid bool var to the thread.
in : extern (Windows) uint thread_entryPoint( void* arg )
obj.m_tls = pstart[0 .. pend - pstart];
+++obj.tlsvalid = true;
POSIX version possiblly need some equivalent fixes either.
Comment #2 by sean — 2009-06-16T21:58:28Z
Regarding:
t = new Thread( (){while(true){}});
delete t;
This code will create and then destroy a thread object because t.start() is never called. Even if you call t.start() however, the delete will occur immediately because it is not preceded by t.join().
As for the TLS issue, Thread.sm_tbeg will be null until thread_init() is called by the GC, so the loop containing the scan(tls) call won't execute before the TLS slice is initialized.
I'm going to mark this ticket as resolved since the issues I addressed above aren't actually problems with the design.
Comment #3 by davidl — 2009-06-17T08:17:19Z
Have you tested the code?
I certainly get the range error in thread_scanAll of accessing m_tls
Comment #4 by davidl — 2009-06-17T08:41:44Z
I don't think let users delete the obj and leave the users to detect what's going wrong later access violation in their threads. This is nasty.
I insist we choose either:
throw an exception or
just hang there if the underlying system thread is still alive.
Comment #5 by sean — 2009-06-17T11:55:33Z
Oh I see. If the array is empty then &m_tls[0] will cause a RangeError. I forget that DMD considers taking the address of an array element a dereferencing operation. Fixed in Druntime revision 164.
Comment #6 by fvbommel — 2009-06-17T12:53:34Z
(In reply to comment #5)
> Oh I see. If the array is empty then &m_tls[0] will cause a RangeError. I
> forget that DMD considers taking the address of an array element a
> dereferencing operation.
It's not the address-taking that is a dereference, it's the indexing (regardless of whether the address of the result is taken). :)
(and if you didn't know, why didn't you use "&t.m_tls[$]" instead of "&t.m_tls[0] + t.m_tls.length"?)
Comment #7 by sean — 2009-06-17T13:09:32Z
That's what I meant :-) I dunno why I didn't use the [$] form though. Probably just an overlooked change during maintenance.
Comment #8 by peter.alexander.au — 2014-02-16T02:57:08Z
"Fixed in Druntime revision 164."
So... is this fixed?
Comment #9 by code — 2014-07-03T00:05:56Z
Closing, as this is an ancient issue and seems to be fixed.