Windows Socket IO causes VM crash

Socket IO on Windows can crash the VM with an access violation due to a race condition on memory freeing in aioWin.c.

The sequence of events that can lead to the crash is:

    allHandles is malloc'd in aioPoll() and passed to sliceWaitForMultipleObjects().
    sliceWaitForMultipleObjects() stores a pointer to allHandles in sliceData->handles.
    waitHandlesThreadFunction() is then called from one or more threads and copies the data from allHandles.
    aioPoll() then waits for an event using WaitForMultipleObjectsEx().
    Once WaitForMultipleObjectsEx() returns it checks the results and frees allHandles.

However this assumes that every thread gets a chance to run prior to WaitForMultipleObjectsEx() returning. But WaitForMultipleObjectsEx() will return either after a timeout or after the first thread indicates an event - since WaitForMultipleObjectsEx() is called with bWaitAll = FALSE.

On a machine with only a couple of cores and a large number of sockets open allHandles can be freed prior to all threads getting enough CPU time, causing an access violation.

The solution is to allocate a buffer per thread and copy the relevant portion of allHandles in to it prior to spawning the thread.

As not all threads get a chance to complete prior to WaitForMultipleObjectsEx() returning, it also means that socket IO may not be recognised, and the socket not read (or written).

Polling all sockets even on timeout ensures that all IO is recognised. The time for checkEventsInHandles() is less than 1mS, so the overhead is minimal, with basically no work being done if no handles are registered for asynchronous IO.

Fixes: pharo-project#495

extracted/vm/src/win/aioWin.c

@@ -372,38 +372,29 @@ struct sliceData {
 DWORD WINAPI waitHandlesThreadFunction(struct sliceData* sliceData ){
 
 	DWORD returnValue;
-	HANDLE* handles;
-	int size;
-	long microSeconds;
 
-	// I copy the data just in case.
-
-	size = sliceData->size;
-	microSeconds = sliceData->microSeconds;
-
-	handles = malloc(sizeof(HANDLE) * size);
-	for(int i = 0; i < size; i++){
-		handles[i] = sliceData->handles[i];
-	}
+	returnValue = WaitForMultipleObjectsEx(sliceData->size, sliceData->handles, FALSE, sliceData->microSeconds / 1000, FALSE);
 
+	free(sliceData->handles);
 	free(sliceData);
-
-	returnValue = WaitForMultipleObjectsEx(size, handles, FALSE, microSeconds / 1000, FALSE);
-
-	free(handles);
 	return 0;
 }
 
 static HANDLE sliceWaitForMultipleObjects(HANDLE* allHandles, int initialIndex, int sizeToProcess, long microSeconds){
 
 	HANDLE r;
+	HANDLE* handles;
 	struct sliceData* sliceData = malloc(sizeof(struct sliceData));
 
 	sliceData->handles = &(allHandles[initialIndex]);
 	sliceData->size = sizeToProcess;
 	sliceData->microSeconds = microSeconds;
-
-//	logTrace("Launching slice from %d size %d", initialIndex, sizeToProcess);
+
+	handles = malloc(sizeof(HANDLE) * sizeToProcess);
+	for(int i = 0; i < sizeToProcess; i++){
+		handles[i] = sliceData->handles[i];
+	}
+	sliceData->handles = handles;
 
 	r = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) waitHandlesThreadFunction, sliceData, 0, NULL);
 
@@ -506,16 +497,6 @@ EXPORT(long) aioPoll(long microSeconds){
 		CloseHandle(waitingHandles[i]);
 	}
 
-
-	if(returnValue == WAIT_TIMEOUT){
-		heartbeat_poll_exit(microSeconds);
-
-		free(waitingHandles);
-		free(allHandles);
-
-		return hasEvents;
-	}
-
 	if(returnValue == WAIT_FAILED){
 		int lastError = GetLastError();
 
@@ -535,15 +516,12 @@ EXPORT(long) aioPoll(long microSeconds){
 	heartbeat_poll_exit(microSeconds);
 
 	/*
-	 * If it is the first is the interrupt event that we use to break the poll.
-	 * If it is interrupted we need to clear the interrupt event
+	 * If interruptEvent was signalled (to interrupt the timeout) we need to clear it.
 	 */
-
 	if(returnValue == WAIT_OBJECT_0){
 		ResetEvent(interruptEvent);
-	}else{
-		hasEvents = checkEventsInHandles(allHandles, size);
 	}
+	hasEvents = checkEventsInHandles(allHandles, size);
 
 	free(waitingHandles);
 	free(allHandles);