[Gc] GC_CreateThread & DuplicateHandle

Boehm, Hans hans_boehm@hp.com
Thu, 24 Jul 2003 17:14:47 -0700


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.)

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?

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

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
>