Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4be4e69f authored by Brad Fitzpatrick's avatar Brad Fitzpatrick
Browse files

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
parent 51101e86

include/sysutils/SocketClient.h

+1 −1
Original line number Diff line number Diff line
@@ -44,7 +44,7 @@ public: 
    // SocketListener creates a SocketClient (at refcount 1) and calls

    // decRef() when it's done with the client.

    void incRef();

    void decRef();

    bool decRef(); // returns true at 0 (but note: SocketClient already deleted)

};



typedef android::List<SocketClient *> SocketClientCollection;

libsysutils/src/SocketClient.cpp

+17 −14
Original line number Diff line number Diff line
@@ -109,15 +109,18 @@ void SocketClient::incRef() { 
    pthread_mutex_unlock(&mRefCountMutex);

}



void SocketClient::decRef() {

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;

}

libsysutils/src/SocketListener.cpp

+5 −2
Original line number Diff line number Diff line
@@ -225,8 +225,11 @@ void SocketListener::runListener() { 
                }

                pthread_mutex_unlock(&mClientsLock);

                /* Destroy the client */

                close(c->getSocket());

                c->decRef();

                int socket = c->getSocket();

                if (c->decRef()) {

                    // Note: 'c' is deleted memory at this point.

                    close(socket);

                }

            }

        }

    }