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 of903b749f+9b587dec, with windows disabled)
This commit is contained in:
parent
72bad95df5
commit
6f641adea5
4 changed files with 180 additions and 45 deletions
|
|
@ -200,7 +200,10 @@ endif
|
||||||
# will violate ODR
|
# will violate ODR
|
||||||
LOCAL_SHARED_LIBRARIES :=
|
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)
|
include $(BUILD_HOST_EXECUTABLE)
|
||||||
|
endif
|
||||||
|
|
||||||
$(call dist-for-goals,dist_files sdk,$(LOCAL_BUILT_MODULE))
|
$(call dist-for-goals,dist_files sdk,$(LOCAL_BUILT_MODULE))
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,6 @@
|
||||||
#ifndef ADB_MUTEX
|
#ifndef ADB_MUTEX
|
||||||
#error ADB_MUTEX not defined when including this file
|
#error ADB_MUTEX not defined when including this file
|
||||||
#endif
|
#endif
|
||||||
ADB_MUTEX(socket_list_lock)
|
|
||||||
ADB_MUTEX(transport_lock)
|
ADB_MUTEX(transport_lock)
|
||||||
#if ADB_HOST
|
#if ADB_HOST
|
||||||
ADB_MUTEX(local_transports_lock)
|
ADB_MUTEX(local_transports_lock)
|
||||||
|
|
|
||||||
|
|
@ -25,18 +25,27 @@
|
||||||
#include <string.h>
|
#include <string.h>
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
|
||||||
|
#include <algorithm>
|
||||||
|
#include <mutex>
|
||||||
|
#include <string>
|
||||||
|
#include <vector>
|
||||||
|
|
||||||
#if !ADB_HOST
|
#if !ADB_HOST
|
||||||
#include "cutils/properties.h"
|
#include "cutils/properties.h"
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
#include "adb.h"
|
#include "adb.h"
|
||||||
#include "adb_io.h"
|
#include "adb_io.h"
|
||||||
|
#include "sysdeps/mutex.h"
|
||||||
#include "transport.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 unsigned local_socket_next_id = 1;
|
||||||
|
|
||||||
static asocket local_socket_list = {
|
static asocket local_socket_list = {
|
||||||
|
|
@ -61,7 +70,7 @@ asocket *find_local_socket(unsigned local_id, unsigned peer_id)
|
||||||
asocket *s;
|
asocket *s;
|
||||||
asocket *result = NULL;
|
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) {
|
for (s = local_socket_list.next; s != &local_socket_list; s = s->next) {
|
||||||
if (s->id != local_id)
|
if (s->id != local_id)
|
||||||
continue;
|
continue;
|
||||||
|
|
@ -70,7 +79,6 @@ asocket *find_local_socket(unsigned local_id, unsigned peer_id)
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
adb_mutex_unlock(&socket_list_lock);
|
|
||||||
|
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
@ -84,20 +92,17 @@ insert_local_socket(asocket* s, asocket* list)
|
||||||
s->next->prev = s;
|
s->next->prev = s;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void install_local_socket(asocket* s) {
|
||||||
void install_local_socket(asocket *s)
|
std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
|
||||||
{
|
|
||||||
adb_mutex_lock(&socket_list_lock);
|
|
||||||
|
|
||||||
s->id = local_socket_next_id++;
|
s->id = local_socket_next_id++;
|
||||||
|
|
||||||
// Socket ids should never be 0.
|
// Socket ids should never be 0.
|
||||||
if (local_socket_next_id == 0)
|
if (local_socket_next_id == 0) {
|
||||||
local_socket_next_id = 1;
|
fatal("local socket id overflow");
|
||||||
|
}
|
||||||
|
|
||||||
insert_local_socket(s, &local_socket_list);
|
insert_local_socket(s, &local_socket_list);
|
||||||
|
|
||||||
adb_mutex_unlock(&socket_list_lock);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void remove_socket(asocket *s)
|
void remove_socket(asocket *s)
|
||||||
|
|
@ -116,19 +121,17 @@ void remove_socket(asocket *s)
|
||||||
void close_all_sockets(atransport *t)
|
void close_all_sockets(atransport *t)
|
||||||
{
|
{
|
||||||
asocket *s;
|
asocket *s;
|
||||||
|
/* this is a little gross, but since s->close() *will* modify
|
||||||
/* this is a little gross, but since s->close() *will* modify
|
** the list out from under you, your options are limited.
|
||||||
** the list out from under you, your options are limited.
|
*/
|
||||||
*/
|
std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
|
||||||
adb_mutex_lock(&socket_list_lock);
|
|
||||||
restart:
|
restart:
|
||||||
for(s = local_socket_list.next; s != &local_socket_list; s = s->next){
|
for (s = local_socket_list.next; s != &local_socket_list; s = s->next) {
|
||||||
if(s->transport == t || (s->peer && s->peer->transport == t)) {
|
if (s->transport == t || (s->peer && s->peer->transport == t)) {
|
||||||
local_socket_close_locked(s);
|
local_socket_close(s);
|
||||||
goto restart;
|
goto restart;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
adb_mutex_unlock(&socket_list_lock);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static int local_socket_enqueue(asocket *s, apacket *p)
|
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);
|
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
|
// be sure to hold the socket list lock when calling this
|
||||||
static void local_socket_destroy(asocket *s)
|
static void local_socket_destroy(asocket *s)
|
||||||
{
|
{
|
||||||
|
|
@ -226,27 +222,21 @@ static void local_socket_destroy(asocket *s)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void local_socket_close(asocket* s) {
|
||||||
static void local_socket_close_locked(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);
|
||||||
D("entered local_socket_close_locked. LS(%d) fd=%d\n", s->id, s->fd);
|
if (s->peer) {
|
||||||
if(s->peer) {
|
D("LS(%d): closing peer. peer->id=%d peer->fd=%d", s->id, s->peer->id, s->peer->fd);
|
||||||
D("LS(%d): closing peer. peer->id=%d peer->fd=%d\n",
|
|
||||||
s->id, s->peer->id, s->peer->fd);
|
|
||||||
/* Note: it's important to call shutdown before disconnecting from
|
/* Note: it's important to call shutdown before disconnecting from
|
||||||
* the peer, this ensures that remote sockets can still get the id
|
* the peer, this ensures that remote sockets can still get the id
|
||||||
* of the local socket they're connected to, to send a CLOSE()
|
* of the local socket they're connected to, to send a CLOSE()
|
||||||
* protocol event. */
|
* protocol event. */
|
||||||
if (s->peer->shutdown)
|
if (s->peer->shutdown) {
|
||||||
s->peer->shutdown(s->peer);
|
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);
|
|
||||||
}
|
}
|
||||||
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
|
/* If we are already closing, or if there are no
|
||||||
|
|
|
||||||
143
adb/sysdeps/mutex.h
Normal file
143
adb/sysdeps/mutex.h
Normal file
|
|
@ -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
|
||||||
Loading…
Add table
Reference in a new issue