adb: win32: fix shutdown deadlock

adb can hang at shutdown due to a deadlock relating to WSACleanup().
This works around the issue by not calling WSACleanup() which shouldn't
be done anyway since threads aren't done using Winsock at shutdown.

A quick way to reproduce the original problem is to run many instances
of adb, many of which will call exit() soon:

  for /l %i in (1,1,20) do @start adb nodaemon server

You may have to boost the 20 to 200, or set ADB_TRACE=1 or use Windows
10 instead of Windows 7, to affect the timing, but eventually there
should be hung adb processes with that repro.

A more complete fix to prevent problems like this from occuring in the
future, would be to additionally do the following:

- Investigate all static destructors that are called when exit() is
  called.

- If they don't do anything important, switch all calls to exit() to
  instead call _exit() and then ban exit() from being called.

Change-Id: Id1be3bf0053809a45f2eca4461e4c35b5ef9388d
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
This commit is contained in:
Spencer Low 2015-08-12 18:19:16 -07:00
parent b81c410abe
commit 87e97ee305
1 changed files with 15 additions and 12 deletions

View File

@ -651,21 +651,11 @@ static int _fh_socket_write(FH f, const void* buf, int len) {
static int _winsock_init;
static void
_cleanup_winsock( void )
{
// TODO: WSAStartup() might be called multiple times and this won't properly
// cleanup the right number of times. Plus, WSACleanup() probably doesn't
// make sense since it might interrupt other threads using Winsock (since
// our various threads are not explicitly cleanly shutdown at process exit).
WSACleanup();
}
static void
_init_winsock( void )
{
// TODO: Multiple threads calling this may potentially cause multiple calls
// to WSAStartup() and multiple atexit() calls.
// to WSAStartup() which offers no real benefit.
if (!_winsock_init) {
WSADATA wsaData;
int rc = WSAStartup( MAKEWORD(2,2), &wsaData);
@ -673,8 +663,21 @@ _init_winsock( void )
fatal( "adb: could not initialize Winsock: %s",
SystemErrorCodeToString( rc ).c_str());
}
atexit( _cleanup_winsock );
_winsock_init = 1;
// Note that we do not call atexit() to register WSACleanup to be called
// at normal process termination because:
// 1) When exit() is called, there are still threads actively using
// Winsock because we don't cleanly shutdown all threads, so it
// doesn't make sense to call WSACleanup() and may cause problems
// with those threads.
// 2) A deadlock can occur when exit() holds a C Runtime lock, then it
// calls WSACleanup() which tries to unload a DLL, which tries to
// grab the LoaderLock. This conflicts with the device_poll_thread
// which holds the LoaderLock because AdbWinApi.dll calls
// setupapi.dll which tries to load wintrust.dll which tries to load
// crypt32.dll which calls atexit() which tries to acquire the C
// Runtime lock that the other thread holds.
}
}