Fix deadlock in spawn driver on windows

A misbehaving port program that does not read all data written to the port
may deadlock the scheduler thread when it calls port_close. The chosen solution
was to use the new function CancelIoEx if it exist (Vista) otherwise let the
spawn driver wait for a short while (10ms) and then to spawn a thread that will
wait for the port program to exit.

2 files changed, 111 insertions, 20 deletions

diff --git a/erts/emulator/sys/win32/sys.c b/erts/emulator/sys/win32/sys.c
index 46dee826f0..65719a91d6 100644
--- a/erts/emulator/sys/win32/sys.c
+++ b/erts/emulator/sys/win32/sys.c
@@ -33,6 +33,7 @@
 #include "../../drivers/win32/win_con.h"
 
 
+
 void erts_sys_init_float(void);
 
 void erl_start(int, char**);
@@ -95,6 +96,10 @@ static erts_smp_mtx_t sys_driver_data_lock;
 static FUNCTION(int, driver_write, (long, HANDLE, byte*, int));
 static void common_stop(int);
 static int create_file_thread(struct async_io* aio, int mode);
+#ifdef ERTS_SMP
+static void close_active_handles(ErlDrvPort, const HANDLE* handles, int cnt);
+static DWORD WINAPI threaded_handle_closer(LPVOID param);
+#endif
 static DWORD WINAPI threaded_reader(LPVOID param);
 static DWORD WINAPI threaded_writer(LPVOID param);
 static DWORD WINAPI threaded_exiter(LPVOID param);
@@ -132,6 +137,9 @@ static BOOL win_console = FALSE;
 
 static OSVERSIONINFO int_os_version;	/* Version information for Win32. */
 
+#ifdef ERTS_SMP
+static BOOL (WINAPI *fpCancelIoEx)(HANDLE,LPOVERLAPPED);
+#endif
 
 /* This is the system's main function (which may or may not be called "main")
    - do general system-dependent initialization
@@ -676,25 +684,50 @@ release_driver_data(DriverData* dp)
     erts_smp_mtx_lock(&sys_driver_data_lock);
 
 #ifdef ERTS_SMP
-    /* This is a workaround for the fact that CancelIo cant cancel
-       requests issued by another thread and that we still cant use
-       CancelIoEx as that's only availabele in Vista etc. */
-    if(dp->in.async_io_active && dp->in.fd != INVALID_HANDLE_VALUE) {
-	CloseHandle(dp->in.fd);
-	dp->in.fd = INVALID_HANDLE_VALUE;
-	DEBUGF(("Waiting for the in event thingie"));
-	WaitForSingleObject(dp->in.ov.hEvent,INFINITE);
-	DEBUGF(("...done\n"));
-    }
-    if(dp->out.async_io_active && dp->out.fd != INVALID_HANDLE_VALUE) {
-	CloseHandle(dp->out.fd);
-	dp->out.fd = INVALID_HANDLE_VALUE;
-	DEBUGF(("Waiting for the out event thingie"));
-	WaitForSingleObject(dp->out.ov.hEvent,INFINITE);
-	DEBUGF(("...done\n"));
+    if (fpCancelIoEx != NULL) {
+	if (dp->in.thread == (HANDLE) -1 && dp->in.fd != INVALID_HANDLE_VALUE) {
+	    (*fpCancelIoEx)(dp->in.fd, NULL);
+	}
+	if (dp->out.thread == (HANDLE) -1 && dp->out.fd != INVALID_HANDLE_VALUE) {
+	    (*fpCancelIoEx)(dp->out.fd, NULL);
+	}
+    }
+    else {
+	/* This is a workaround for the fact that CancelIo cant cancel
+	   requests issued by another thread and that we cant use
+	   CancelIoEx as that's only availabele in Vista etc.
+	   R14: Avoid scheduler deadlock by only wait for 10ms, and then spawn
+	    a thread that will keep waiting in in order to close handles. */
+	HANDLE handles[2];
+	int i = 0;
+	int timeout = 10;
+	if(dp->in.async_io_active && dp->in.fd != INVALID_HANDLE_VALUE) {
+	    CloseHandle(dp->in.fd);
+	    dp->in.fd = INVALID_HANDLE_VALUE;
+	    DEBUGF(("Waiting for the in event thingie"));
+	    if (WaitForSingleObject(dp->in.ov.hEvent,timeout) == WAIT_TIMEOUT) {
+		handles[i++] = dp->in.ov.hEvent;
+		dp->in.ov.hEvent = NULL;
+		timeout = 0;
+	    }
+	    DEBUGF(("...done\n"));
+	}
+	if(dp->out.async_io_active && dp->out.fd != INVALID_HANDLE_VALUE) {
+	    CloseHandle(dp->out.fd);
+	    dp->out.fd = INVALID_HANDLE_VALUE;
+	    DEBUGF(("Waiting for the out event thingie"));
+	    if (WaitForSingleObject(dp->out.ov.hEvent,timeout) == WAIT_TIMEOUT) {
+		handles[i++] = dp->out.ov.hEvent;
+		dp->out.ov.hEvent = NULL;
+	    }
+	    DEBUGF(("...done\n"));
+	}
+	if (i > 0) {
+	    close_active_handles(dp->port_num, handles, i);
+	}
     }
 #else
-    	if (dp->out.thread == (HANDLE) -1 && dp->in.fd != INVALID_HANDLE_VALUE) {
+	if (dp->in.thread == (HANDLE) -1 && dp->in.fd != INVALID_HANDLE_VALUE) {
 	     CancelIo(dp->in.fd); 
 	}
 	if (dp->out.thread == (HANDLE) -1 && dp->out.fd != INVALID_HANDLE_VALUE) {
@@ -737,6 +770,48 @@ release_driver_data(DriverData* dp)
     erts_smp_mtx_unlock(&sys_driver_data_lock);
 }
 
+#ifdef ERTS_SMP
+
+struct handles_to_be_closed
+{
+    int cnt;
+    HANDLE handles[2];
+};
+
+static void close_active_handles(ErlDrvPort port_num, const HANDLE* handles, int cnt)
+{
+    DWORD tid;
+    HANDLE thread;
+    int i;
+    struct handles_to_be_closed* htbc = erts_alloc(ERTS_ALC_T_DRV_TAB,
+						   sizeof(struct handles_to_be_closed));
+    htbc->cnt = cnt;
+    for (i=0; i < cnt; ++i) {
+	htbc->handles[i] = handles[i];
+	(void) driver_select(port_num, (ErlDrvEvent)handles[i],
+			     ERL_DRV_USE_NO_CALLBACK, 0);
+    }
+    thread = (HANDLE *) _beginthreadex(NULL, 0, threaded_handle_closer, htbc, 0, &tid);
+    CloseHandle(thread);
+}
+
+
+static DWORD WINAPI
+threaded_handle_closer(LPVOID param)
+{
+    struct handles_to_be_closed* htbc = (struct handles_to_be_closed*) param;
+    int i;
+    DEBUGF(("threaded_handle_closer waiting for %d handles\r\n",htbc->cnt));
+    WaitForMultipleObjects(htbc->cnt, htbc->handles, TRUE, INFINITE);
+    for (i=0; i < htbc->cnt; ++i) {
+	CloseHandle(htbc->handles[i]);
+    }
+    erts_free(ERTS_ALC_T_DRV_TAB, htbc);
+    DEBUGF(("threaded_handle_closer terminating\r\n"));
+    return 0;
+}
+#endif /* ERTS_SMP */
+
 /*
  * Stores input and output file descriptors in the DriverData structure,
  * and calls driver_select().
@@ -1026,12 +1101,19 @@ static int
 spawn_init()
 {
     int i;
-  
+#ifdef ERTS_SMP
+    HMODULE module = GetModuleHandle("kernel32");
+    fpCancelIoEx = (module != NULL) ?
+	(BOOL (WINAPI *)(HANDLE,LPOVERLAPPED))
+      GetProcAddress(module,"CancelIoEx") : NULL;
+    DEBUGF(("fpCancelIoEx = %p\r\n", fpCancelIoEx));
+#endif
     driver_data = (struct driver_data *)
 	erts_alloc(ERTS_ALC_T_DRV_TAB, max_files * sizeof(struct driver_data));
     erts_smp_atomic_add(&sys_misc_mem_sz, max_files*sizeof(struct driver_data));
     for (i = 0; i < max_files; i++)
 	driver_data[i].port_num = PORT_FREE;
+
     return 0;
 }
 
diff --git a/erts/emulator/test/port_SUITE.erl b/erts/emulator/test/port_SUITE.erl
index eb69bf917b..66aff307a3 100644
--- a/erts/emulator/test/port_SUITE.erl
+++ b/erts/emulator/test/port_SUITE.erl
@@ -88,7 +88,7 @@
 	 otp_3906/1, otp_4389/1, win_massive/1, win_massive_client/1,
 	 mix_up_ports/1, otp_5112/1, otp_5119/1, otp_6224/1,
 	 exit_status_multi_scheduling_block/1, ports/1,
-	 spawn_driver/1,spawn_executable/1,
+	 spawn_driver/1, spawn_executable/1, close_deaf_port/1,
 	 unregister_name/1]).
 
 -export([]).
@@ -113,7 +113,7 @@ all(suite) ->
      otp_3906, otp_4389, win_massive, mix_up_ports,
      otp_5112, otp_5119,
      exit_status_multi_scheduling_block,
-     ports, spawn_driver, spawn_executable,
+     ports, spawn_driver, spawn_executable, close_deaf_port,
      unregister_name
     ].
 
@@ -2293,3 +2293,12 @@ load_driver(Dir, Driver) ->
 	    io:format("~s\n", [erl_ddll:format_error(Error)]),
 	    Res
     end.
+
+
+close_deaf_port(doc) -> ["Send data to port program that does not read it, then close port."];
+close_deaf_port(suite) -> [];
+close_deaf_port(Config) when is_list(Config) ->
+    Port = open_port({spawn,"sleep 999999"},[]),
+    erlang:port_command(Port,"Hello, can you hear me!?!?"),
+    port_close(Port),
+    ok.