Fix potential race introduced in Icd7f5f03
Digit wrote:
"You probably don't want to close the socket here without updating
c->socket as well. Otherwise, another thread holding a handle to the
client after the c->decRef() could end up sending a message to a
different socket, if the file descriptor index is reused by another
client in the meantime."
Change-Id: Icdefb5ffc0c7607325d7db761e1f04e5d868bfb7
diff --git a/libsysutils/src/SocketClient.cpp b/libsysutils/src/SocketClient.cpp
index 6d4dff4..90ca52e 100644
--- a/libsysutils/src/SocketClient.cpp
+++ b/libsysutils/src/SocketClient.cpp
@@ -104,20 +104,23 @@
}
void SocketClient::incRef() {
- pthread_mutex_lock(&mRefCountMutex);
- mRefCount++;
- pthread_mutex_unlock(&mRefCountMutex);
+ pthread_mutex_lock(&mRefCountMutex);
+ mRefCount++;
+ pthread_mutex_unlock(&mRefCountMutex);
}
-void SocketClient::decRef() {
- bool deleteSelf = false;
- pthread_mutex_lock(&mRefCountMutex);
- mRefCount--;
- if (mRefCount == 0) {
- deleteSelf = true;
- }
- pthread_mutex_unlock(&mRefCountMutex);
- if (deleteSelf) {
- delete this;
- }
+bool SocketClient::decRef() {
+ bool deleteSelf = false;
+ pthread_mutex_lock(&mRefCountMutex);
+ mRefCount--;
+ if (mRefCount == 0) {
+ deleteSelf = true;
+ } else if (mRefCount < 0) {
+ SLOGE("SocketClient refcount went negative!");
+ }
+ pthread_mutex_unlock(&mRefCountMutex);
+ if (deleteSelf) {
+ delete this;
+ }
+ return deleteSelf;
}