コミット: 11440395e379f35069e8e39a6e3ccfa7a7bfc72f - system-coremmmm (git)

system/corennnnn

コミットメタ情報

• リビジョン: 11440395e379f35069e8e39a6e3ccfa7a7bfc72f (tree)
• 日時: 2016-08-17 07:52:46
• 作者: Josh Gao <jmgao@goog...>
• コミッター: gitbuildkicker

ログメッセージ

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)

変更サマリ

• modified: adb/Android.mk
• modified: adb/mutex_list.h
• modified: adb/sockets.cpp
• add: adb/sysdeps/mutex.h

差分

--- a/adb/Android.mk
+++ b/adb/Android.mk
@@ -200,7 +200,10 @@ endif
 # 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))
 

--- 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)

--- 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 *find_local_socket(unsigned local_id, unsigned peer_id)
     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 @@ asocket *find_local_socket(unsigned local_id, unsigned peer_id)
         }
         break;
     }
-    adb_mutex_unlock(&socket_list_lock);
 
     return result;
 }
@@ -84,20 +92,17 @@ insert_local_socket(asocket*  s, asocket*  list)
     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 remove_socket(asocket *s)
 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 @@ static void local_socket_ready(asocket *s)
     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_destroy(asocket  *s)
     }
 }
 
-
-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

--- /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