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

Commit a997b239 authored by Hans Boehm's avatar Hans Boehm
Browse files

Make concurrently accessed globals atomic 

Eliminate data races on gHaveTLS, gShutdown, and
gDisableBackgroundScheduling by declaring them as atomic. Use
explicit load() and store() operations to access them.

Note that gHaveTLS requires memory ordering guarantees, and thus,
even with a cooperative compiler, the old code would never have been
compiled to correct ARM object code.

Document a couple of weirdnesses remaining in the code.

Bug: 36697681
Test: Boot AOSP, TreeHugger
Change-Id: If44a10e31c1e091f06179154e07ced3da0a37b68
parent 6fea53e6

libs/binder/IPCThreadState.cpp

+15 −13
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ 
#include <private/binder/binder_module.h>

#include <private/binder/Static.h>



#include <atomic>

#include <errno.h>

#include <inttypes.h>

#include <pthread.h>
@@ -276,14 +277,14 @@ static const void* printCommand(TextOutput& out, const void* _cmd) 
}



static pthread_mutex_t gTLSMutex = PTHREAD_MUTEX_INITIALIZER;

static bool gHaveTLS = false;

static std::atomic<bool> gHaveTLS(false);

static pthread_key_t gTLS = 0;

static bool gShutdown = false;

static bool gDisableBackgroundScheduling = false;

static std::atomic<bool> gShutdown = false;

static std::atomic<bool> gDisableBackgroundScheduling = false;



IPCThreadState* IPCThreadState::self()

{

    if (gHaveTLS) {

    if (gHaveTLS.load(std::memory_order_acquire)) {

restart:

        const pthread_key_t k = gTLS;

        IPCThreadState* st = (IPCThreadState*)pthread_getspecific(k);
@@ -291,13 +292,14 @@ restart: 
        return new IPCThreadState;

    }



    if (gShutdown) {

    // Racey, heuristic test for simultaneous shutdown.

    if (gShutdown.load(std::memory_order_relaxed)) {

        ALOGW("Calling IPCThreadState::self() during shutdown is dangerous, expect a crash.\n");

        return nullptr;

    }



    pthread_mutex_lock(&gTLSMutex);

    if (!gHaveTLS) {

    if (!gHaveTLS.load(std::memory_order_relaxed)) {

        int key_create_value = pthread_key_create(&gTLS, threadDestructor);

        if (key_create_value != 0) {

            pthread_mutex_unlock(&gTLSMutex);
@@ -305,7 +307,7 @@ restart: 
                    strerror(key_create_value));

            return nullptr;

        }

        gHaveTLS = true;

        gHaveTLS.store(true, std::memory_order_release);

    }

    pthread_mutex_unlock(&gTLSMutex);

    goto restart;
@@ -313,7 +315,7 @@ restart: 


IPCThreadState* IPCThreadState::selfOrNull()

{

    if (gHaveTLS) {

    if (gHaveTLS.load(std::memory_order_acquire)) {

        const pthread_key_t k = gTLS;

        IPCThreadState* st = (IPCThreadState*)pthread_getspecific(k);

        return st;
@@ -323,9 +325,9 @@ IPCThreadState* IPCThreadState::selfOrNull() 


void IPCThreadState::shutdown()

{

    gShutdown = true;

    gShutdown.store(true, std::memory_order_relaxed);



    if (gHaveTLS) {

    if (gHaveTLS.load(std::memory_order_acquire)) {

        // XXX Need to wait for all thread pool threads to exit!

        IPCThreadState* st = (IPCThreadState*)pthread_getspecific(gTLS);

        if (st) {
@@ -333,18 +335,18 @@ void IPCThreadState::shutdown() 
            pthread_setspecific(gTLS, nullptr);

        }

        pthread_key_delete(gTLS);

        gHaveTLS = false;

        gHaveTLS.store(false, std::memory_order_release);

    }

}



void IPCThreadState::disableBackgroundScheduling(bool disable)

{

    gDisableBackgroundScheduling = disable;

    gDisableBackgroundScheduling.store(disable, std::memory_order_relaxed);

}



bool IPCThreadState::backgroundSchedulingDisabled()

{

    return gDisableBackgroundScheduling;

    return gDisableBackgroundScheduling.load(std::memory_order_relaxed);

}



sp<ProcessState> IPCThreadState::process()

libs/binder/include/binder/ProcessState.h

+2 −0
Original line number Diff line number Diff line
@@ -124,6 +124,8 @@ private: 
            int64_t             mStarvationStartTimeMs;



    mutable Mutex               mLock;  // protects everything below.

            // TODO: mManagesContexts is often accessed without the lock.

            //       Explain why that's safe.



            Vector<handle_entry>mHandleToObject;