From cb42b3d86e131f6d3ba883055990bb4164e0d6a6 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 28 Apr 2010 20:24:48 +0200 Subject: 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. --- erts/emulator/sys/win32/sys.c | 118 ++++++++++++++++++++++++++++++++------ erts/emulator/test/port_SUITE.erl | 13 ++++- 2 files changed, 111 insertions(+), 20 deletions(-) (limited to 'erts') 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. -- cgit v1.2.3