diff Python/thread_nt.h @ 29755:59da5b57c61b legacy-trunk

An Anonymous Coward on c.l.py posted a little program with bizarre behavior, creating many threads very quickly. A long debugging session revealed that the Windows implementation of PyThread_start_new_thread() was choked with "laziness" errors: 1. It checked MS _beginthread() for a failure return, but when that happened it returned heap trash as the function result, instead of an id of -1 (the proper error-return value). 2. It didn't consider that the Win32 CreateSemaphore() can fail. 3. When creating a great many threads very quickly, it's quite possible that any particular bootstrap call can take virtually any amount of time to return. But the code waited for a maximum of 5 seconds, and didn't check to see whether the semaphore it was waiting for got signaled. If it in fact timed out, the function could again return heap trash as the function result. This is actually what confused the test program, as the heap trash usually turned out to be 0, and then multiple threads all got id 0 simultaneously, confusing the hell out of threading.py's _active dict (mapping id to thread object). A variety of baffling behaviors followed from that. WRT #1 and #2, error returns are checked now, and "thread.error: can't start new thread" gets raised now if a new thread (or new semaphore) can't be created. WRT #3, we now wait for the semaphore without a timeout. Also removed useless local vrbls, folded long lines, and changed callobj to a stack auto (it was going thru malloc/free instead, for no discernible reason). Bugfix candidate.
author Tim Peters <tim.peters@gmail.com>
date Fri, 04 Jul 2003 04:40:45 +0000
parents 7a3e03b541ff
children 9241218992c1
line wrap: on
line diff
--- a/Python/thread_nt.h	Thu Jul 03 21:33:02 2003 +0000
+++ b/Python/thread_nt.h	Fri Jul 04 04:40:45 2003 +0000
@@ -148,7 +148,7 @@
 
 typedef struct {
 	void (*func)(void*);
-	void *arg;			
+	void *arg;
 	long id;
 	HANDLE done;
 } callobj;
@@ -167,35 +167,42 @@
 	return 0;
 }
 
-long PyThread_start_new_thread(void (*func)(void *), void *arg)
+long
+PyThread_start_new_thread(void (*func)(void *), void *arg)
 {
 	unsigned long rv;
-	int success = 0;
-	callobj *obj;
-	int id;
+	callobj obj;
 
-	dprintf(("%ld: PyThread_start_new_thread called\n", PyThread_get_thread_ident()));
+	dprintf(("%ld: PyThread_start_new_thread called\n",
+		 PyThread_get_thread_ident()));
 	if (!initialized)
 		PyThread_init_thread();
 
-	obj = malloc(sizeof(callobj)); 
-	obj->func = func;
-	obj->arg = arg;
-	obj->done = CreateSemaphore(NULL, 0, 1, NULL);
+	obj.id = -1;	/* guilty until proved innocent */
+	obj.func = func;
+	obj.arg = arg;
+	obj.done = CreateSemaphore(NULL, 0, 1, NULL);
+	if (obj.done == NULL)
+		return -1;
 
-	rv = _beginthread(bootstrap, 0, obj); /* use default stack size */
- 
-	if (rv != (unsigned long)-1) {
-		success = 1;
-		dprintf(("%ld: PyThread_start_new_thread succeeded: %p\n", PyThread_get_thread_ident(), rv));
+	rv = _beginthread(bootstrap, 0, &obj); /* use default stack size */
+	if (rv == (unsigned long)-1) {
+		/* I've seen errno == EAGAIN here, which means "there are
+		 * too many threads".
+		 */
+		dprintf(("%ld: PyThread_start_new_thread failed: %p errno %d\n",
+		         PyThread_get_thread_ident(), rv, errno));
+		obj.id = -1;
 	}
-
-	/* wait for thread to initialize and retrieve id */
-	WaitForSingleObject(obj->done, 5000);  /* maybe INFINITE instead of 5000? */
-	CloseHandle((HANDLE)obj->done);
-	id = obj->id;
-	free(obj);
-	return id;
+	else {
+		dprintf(("%ld: PyThread_start_new_thread succeeded: %p\n",
+		         PyThread_get_thread_ident(), rv));
+		/* wait for thread to initialize, so we can get its id */
+		WaitForSingleObject(obj.done, INFINITE);
+		assert(obj.id != -1);
+	}
+	CloseHandle((HANDLE)obj.done);
+	return obj.id;
 }
 
 /*