adb: switch the socket list mutex to a recursive_mutex.
sockets.cpp was branching on whether a socket close function was
local_socket_close in order to avoid a potential deadlock if the socket
list lock was held while closing a peer socket.
Bug: http://b/28347842
Change-Id: I5e56f17fa54275284787f0f1dc150d1960256ab3
(functionally a cherrypick of 903b749f + 9b587dec, with windows disabled)
diff --git a/adb/Android.mk b/adb/Android.mk
index 425bf9b..d4adbfb 100644
--- a/adb/Android.mk
+++ b/adb/Android.mk
@@ -200,7 +200,10 @@
# will violate ODR
LOCAL_SHARED_LIBRARIES :=
+# Don't build the host adb on Windows (this branch should only be used for security updates.)
+ifneq ($(HOST_OS),windows)
include $(BUILD_HOST_EXECUTABLE)
+endif
$(call dist-for-goals,dist_files sdk,$(LOCAL_BUILT_MODULE))
diff --git a/adb/mutex_list.h b/adb/mutex_list.h
index ff72751..15e383c 100644
--- a/adb/mutex_list.h
+++ b/adb/mutex_list.h
@@ -6,7 +6,6 @@
#ifndef ADB_MUTEX
#error ADB_MUTEX not defined when including this file
#endif
-ADB_MUTEX(socket_list_lock)
ADB_MUTEX(transport_lock)
#if ADB_HOST
ADB_MUTEX(local_transports_lock)
diff --git a/adb/sockets.cpp b/adb/sockets.cpp
index 32ca17d..31b5443 100644
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -25,18 +25,27 @@
#include <string.h>
#include <unistd.h>
+#include <algorithm>
+#include <mutex>
+#include <string>
+#include <vector>
+
#if !ADB_HOST
#include "cutils/properties.h"
#endif
#include "adb.h"
#include "adb_io.h"
+#include "sysdeps/mutex.h"
#include "transport.h"
-ADB_MUTEX_DEFINE( socket_list_lock );
+#if !defined(__BIONIC__)
+using std::recursive_mutex;
+#endif
-static void local_socket_close_locked(asocket *s);
+static void local_socket_close(asocket* s);
+static recursive_mutex& local_socket_list_lock = *new recursive_mutex();
static unsigned local_socket_next_id = 1;
static asocket local_socket_list = {
@@ -61,7 +70,7 @@
asocket *s;
asocket *result = NULL;
- adb_mutex_lock(&socket_list_lock);
+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
for (s = local_socket_list.next; s != &local_socket_list; s = s->next) {
if (s->id != local_id)
continue;
@@ -70,7 +79,6 @@
}
break;
}
- adb_mutex_unlock(&socket_list_lock);
return result;
}
@@ -84,20 +92,17 @@
s->next->prev = s;
}
-
-void install_local_socket(asocket *s)
-{
- adb_mutex_lock(&socket_list_lock);
+void install_local_socket(asocket* s) {
+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
s->id = local_socket_next_id++;
// Socket ids should never be 0.
- if (local_socket_next_id == 0)
- local_socket_next_id = 1;
+ if (local_socket_next_id == 0) {
+ fatal("local socket id overflow");
+ }
insert_local_socket(s, &local_socket_list);
-
- adb_mutex_unlock(&socket_list_lock);
}
void remove_socket(asocket *s)
@@ -116,19 +121,17 @@
void close_all_sockets(atransport *t)
{
asocket *s;
-
- /* this is a little gross, but since s->close() *will* modify
- ** the list out from under you, your options are limited.
- */
- adb_mutex_lock(&socket_list_lock);
+ /* this is a little gross, but since s->close() *will* modify
+ ** the list out from under you, your options are limited.
+ */
+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
restart:
- for(s = local_socket_list.next; s != &local_socket_list; s = s->next){
- if(s->transport == t || (s->peer && s->peer->transport == t)) {
- local_socket_close_locked(s);
+ for (s = local_socket_list.next; s != &local_socket_list; s = s->next) {
+ if (s->transport == t || (s->peer && s->peer->transport == t)) {
+ local_socket_close(s);
goto restart;
}
}
- adb_mutex_unlock(&socket_list_lock);
}
static int local_socket_enqueue(asocket *s, apacket *p)
@@ -191,13 +194,6 @@
fdevent_add(&s->fde, FDE_READ);
}
-static void local_socket_close(asocket *s)
-{
- adb_mutex_lock(&socket_list_lock);
- local_socket_close_locked(s);
- adb_mutex_unlock(&socket_list_lock);
-}
-
// be sure to hold the socket list lock when calling this
static void local_socket_destroy(asocket *s)
{
@@ -226,27 +222,21 @@
}
}
-
-static void local_socket_close_locked(asocket *s)
-{
- D("entered local_socket_close_locked. LS(%d) fd=%d\n", s->id, s->fd);
- if(s->peer) {
- D("LS(%d): closing peer. peer->id=%d peer->fd=%d\n",
- s->id, s->peer->id, s->peer->fd);
+static void local_socket_close(asocket* s) {
+ D("entered local_socket_close. LS(%d) fd=%d", s->id, s->fd);
+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
+ if (s->peer) {
+ D("LS(%d): closing peer. peer->id=%d peer->fd=%d", s->id, s->peer->id, s->peer->fd);
/* Note: it's important to call shutdown before disconnecting from
* the peer, this ensures that remote sockets can still get the id
* of the local socket they're connected to, to send a CLOSE()
* protocol event. */
- if (s->peer->shutdown)
- s->peer->shutdown(s->peer);
- s->peer->peer = 0;
- // tweak to avoid deadlock
- if (s->peer->close == local_socket_close) {
- local_socket_close_locked(s->peer);
- } else {
- s->peer->close(s->peer);
+ if (s->peer->shutdown) {
+ s->peer->shutdown(s->peer);
}
- s->peer = 0;
+ s->peer->peer = nullptr;
+ s->peer->close(s->peer);
+ s->peer = nullptr;
}
/* If we are already closing, or if there are no
diff --git a/adb/sysdeps/mutex.h b/adb/sysdeps/mutex.h
new file mode 100644
index 0000000..ef5d9b1
--- /dev/null
+++ b/adb/sysdeps/mutex.h
@@ -0,0 +1,143 @@
+#pragma once
+
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#if defined(_WIN32)
+
+#include <windows.h>
+
+#include <android-base/macros.h>
+
+#include "adb.h"
+
+// The prebuilt version of mingw we use doesn't support mutex or recursive_mutex.
+// Therefore, implement our own using the Windows primitives.
+// Put them directly into the std namespace, so that when they're actually available, the build
+// breaks until they're removed.
+
+#include <mutex>
+namespace std {
+
+// CRITICAL_SECTION is recursive, so just wrap it in a Mutex-compatible class.
+class recursive_mutex {
+ public:
+ recursive_mutex() {
+ InitializeCriticalSection(&mutex_);
+ }
+
+ ~recursive_mutex() {
+ DeleteCriticalSection(&mutex_);
+ }
+
+ void lock() {
+ EnterCriticalSection(&mutex_);
+ }
+
+ bool try_lock() {
+ return TryEnterCriticalSection(&mutex_);
+ }
+
+ void unlock() {
+ LeaveCriticalSection(&mutex_);
+ }
+
+ private:
+ CRITICAL_SECTION mutex_;
+
+ DISALLOW_COPY_AND_ASSIGN(recursive_mutex);
+};
+
+class mutex {
+ public:
+ mutex() {
+ }
+
+ ~mutex() {
+ }
+
+ void lock() {
+ mutex_.lock();
+ if (++lock_count_ != 1) {
+ fatal("non-recursive mutex locked reentrantly");
+ }
+ }
+
+ void unlock() {
+ if (--lock_count_ != 0) {
+ fatal("non-recursive mutex unlock resulted in unexpected lock count: %d", lock_count_);
+ }
+ mutex_.unlock();
+ }
+
+ bool try_lock() {
+ if (!mutex_.try_lock()) {
+ return false;
+ }
+
+ if (lock_count_ != 0) {
+ mutex_.unlock();
+ return false;
+ }
+
+ ++lock_count_;
+ return true;
+ }
+
+ private:
+ recursive_mutex mutex_;
+ size_t lock_count_ = 0;
+};
+
+}
+
+#elif defined(__BIONIC__)
+
+// On M, the recovery image uses parts of adb that depends on recursive_mutex, and uses libstdc++,
+// which lacks it.
+
+#include <pthread.h>
+#include <mutex>
+
+#include <base/macros.h>
+
+class recursive_mutex {
+ public:
+ recursive_mutex() {
+ }
+
+ ~recursive_mutex() {
+ }
+
+ void lock() {
+ pthread_mutex_lock(&mutex_);
+ }
+
+ bool try_lock() {
+ return pthread_mutex_trylock(&mutex_);
+ }
+
+ void unlock() {
+ pthread_mutex_unlock(&mutex_);
+ }
+
+ private:
+ pthread_mutex_t mutex_ = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+
+ DISALLOW_COPY_AND_ASSIGN(recursive_mutex);
+};
+
+#endif
