[Gc] GC_CreateThread & DuplicateHandle

Thong (Tum) Nguyen tum@veridicus.com
Fri, 25 Jul 2003 13:58:28 +1200


> -----Original Message-----
> From: gc-admin@napali.hpl.hp.com [mailto:gc-admin@napali.hpl.hp.com]
On
> Behalf Of Boehm, Hans
> Sent: Friday, 25 July 2003 12:15 p.m.
> To: 'Thong (Tum) Nguyen'; gc@napali.hpl.hp.com
> Subject: RE: [Gc] GC_CreateThread & DuplicateHandle
> 
> Thanks.
> 
> The patch looks correct to me, except that the open brace on the line
with
> DUPLICATE_SAME_ACCESS should be a right parenthesis?  I'll put the
fixed
> version in my tree.  (Diff -u output would be easier next time.)

Sorry, I'll diff -u next time.  I'm not sure why that open brace was
there "whoops".  GCC didn't complain about it either.  Weird.

> 
> My reading of the original code is that CloseHandle was already being
called
> everywhere on all the thread handles in the table, in spite of the
fact that they were
> not being duplicated.  Is this also your reading? Apparently closing a
thread handle
> twice is somewhat benign?

That's what I'm reading too.  Closing a thread handle twice causes the
"Suspend Thread failed" errors.  By duplicating the thread handle, the
caller of GC_CreateThread can call CloseHandle on the duplicate without
actually closing the original used by libgc.

Essentially we want to be able to call CloseHandle on a thread inside
libgc and also allow CloseHandle to be called outside of libgc.  The
easiest way to do that is to return a duplicate handle.

> 
> (I'm trying to convince myself that no leaks are being introduced
here.  My
> understanding of the win32 API is limited.)

I'm pretty sure it won't introduce the leak as long as callers of
GC_CreateThread remember to call CloseHandle on the returned handle.

Most people call CloseHandle long after the thread has died, which means
the GC is the one that gets to call CloseHandle first.  The second
CloseHandle call done outside libgc would fail, but since most people
don't actually check the return result of CloseHandle it wouldn't be a
big deal.  The problem occurs if people call CloseHandle before a thread
has finished (which is a valid thing to do).  Without a duplicate
handle, the call would close and invalidate the original handle used
internally by libgc and cause GC_stop_world to fail.

All the best,

^Tum

> 
> Hans
> 
> > -----Original Message-----
> > From: Thong (Tum) Nguyen [mailto:tum@veridicus.com]
> > Sent: Tuesday, July 15, 2003 12:47 PM
> > To: gc@napali.hpl.hp.com
> > Subject: [Gc] GC_CreateThread & DuplicateHandle
> >
> >
> > Hi,
> >
> > It looks like the Win32 GC_CreateThread wrapper for non-DLLs isn't
> > returning a duplicate handle.  This means that the GC will sometimes
> > give a "Suspend Thread failed" error message if the caller closes
the
> > handle before the thread returns (which is perfectly valid).
> >
> > Here's the fix for GC_CreateThread (6.2a6):
> >
> >
> > HANDLE WINAPI GC_CreateThread(
> >     LPSECURITY_ATTRIBUTES lpThreadAttributes,
> >     DWORD dwStackSize, LPTHREAD_START_ROUTINE lpStartAddress,
> >     LPVOID lpParameter, DWORD dwCreationFlags, LPDWORD lpThreadId )
> > {
> >     HANDLE thread_h = NULL;
> >     HANDLE child_ready_h, parent_ready_h;
> >
> >     int i;
> >     thread_args args;
> >
> >     /* allocate thread slot */
> >     LOCK();
> >     for (i = 0; i != MAX_THREADS && thread_table[i].in_use; i++)
> > 	;
> >     if (i != MAX_THREADS) {
> > 	thread_table[i].in_use = TRUE;
> >     }
> >     UNLOCK();
> >
> >     if (i != MAX_THREADS) {
> >
> > 	/* create unnamed unsignalled events */
> > 	if (child_ready_h = CreateEvent(NULL, FALSE, FALSE, NULL)) {
> > 	    if (parent_ready_h = CreateEvent(NULL, FALSE, FALSE, NULL))
> > {
> >
> > 		/* set up thread arguments */
> > 		args.child_ready_h = child_ready_h;
> > 		args.parent_ready_h = parent_ready_h;
> > 		args.entry = &thread_table[i];
> > 		args.start = lpStartAddress;
> > 		args.param = lpParameter;
> >
> > 		thread_h = CreateThread(lpThreadAttributes,
> > 					dwStackSize, thread_start,
> > 					&args,
> > 					dwCreationFlags &
> > ~CREATE_SUSPENDED,
> > 					lpThreadId);
> >
> > 		if (thread_h) {
> >
> > 		    /* fill in ID and handle; tell child this is done */
> > 		    thread_table[i].id = *lpThreadId;
> >
> > 			if (!DuplicateHandle(GetCurrentProcess(),
> > 				thread_h,
> > 				GetCurrentProcess(),
> > 				&thread_table[i].handle,
> > 				0,
> > 				0,
> > 				DUPLICATE_SAME_ACCESS) {
> > 			{
> > 				DWORD last_error = GetLastError();
> > 				GC_printf1("Last error code: %lx\n",
> > last_error);
> > 				ABORT("DuplicateHandle failed");
> >
> > 			}
> >
> > 		    SetEvent (parent_ready_h);
> >
> > 		    /* wait for child to fill in stack and copy args */
> > 		    WaitForSingleObject (child_ready_h, INFINITE);
> >
> > 		    /* suspend the child if requested */
> > 		    if (dwCreationFlags & CREATE_SUSPENDED)
> > 			SuspendThread (thread_table[i].handle);
> >
> > 		    /* let child call given function now (or when
> > resumed) */
> > 		    SetEvent (parent_ready_h);
> >
> > 		} else {
> > 		    CloseHandle (parent_ready_h);
> > 		}
> > 	    }
> > 	}
> >
> > 	CloseHandle (child_ready_h);
> >
> > 	if (thread_h == NULL)
> > 	    thread_table[i].in_use = FALSE;
> >
> >     } else { /* no thread slot found */
> > 	SetLastError (ERROR_TOO_MANY_TCBS);
> >     }
> >
> >     return thread_h;
> > }
> >
> >
> > Thanks,
> >
> > ^Tum
> >
> >
> >
> > _______________________________________________
> > Gc mailing list
> > Gc@linux.hpl.hp.com
> > http://linux.hpl.hp.com/cgi-bin/mailman/listinfo/gc
> >
> _______________________________________________
> Gc mailing list
> Gc@linux.hpl.hp.com
> http://linux.hpl.hp.com/cgi-bin/mailman/listinfo/gc