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

Commit 5576a555 authored by Stephen Kiazyk's avatar Stephen Kiazyk
Browse files

Avoid a potential race condition on mDisplays 

The race could occur when transitioning in/out of VR flinger mode.
It is now avoided by ensuring that the primary |DisplayDevice| is always
created once |mStateLock| is released, and ensuring that all accesses
to the primary |DisplayDevice| are guarded by |mStateLock|.

Bug: 36194616
Test: Normal behavior is unchanged. Explicitly testing the race
condition required instrumenting code with sleep statements.

Change-Id: I1f8fcf23982c311d47267beca8127b6c29562ce5
parent cde29667

services/surfaceflinger/DisplayHardware/HWComposer.cpp

+1 −1
Original line number Diff line number Diff line
@@ -206,7 +206,7 @@ void HWComposer::hotplug(const std::shared_ptr<HWC2::Display>& display, 
        }

        disp = DisplayDevice::DISPLAY_EXTERNAL;

    }

    mEventHandler->onHotplugReceived(disp,

    mEventHandler->onHotplugReceived(this, disp,

            connected == HWC2::Connection::Connected);

}

services/surfaceflinger/DisplayHardware/HWComposer.h

+1 −1
Original line number Diff line number Diff line
@@ -70,7 +70,7 @@ public: 
        friend class HWComposer;

        virtual void onVSyncReceived(

            HWComposer* composer, int32_t disp, nsecs_t timestamp) = 0;

        virtual void onHotplugReceived(int32_t disp, bool connected) = 0;

        virtual void onHotplugReceived(HWComposer* composer, int32_t disp, bool connected) = 0;

        virtual void onInvalidateReceived(HWComposer* composer) = 0;

    protected:

        virtual ~EventHandler() {}

services/surfaceflinger/SurfaceFlinger.cpp

+99 −72
Original line number Diff line number Diff line
@@ -598,7 +598,7 @@ void SurfaceFlinger::init() { 


    // make the GLContext current so that we can create textures when creating

    // Layers (which may happens before we render something)

    getDefaultDisplayDevice()->makeCurrent(mEGLDisplay, mEGLContext);

    getDefaultDisplayDeviceLocked()->makeCurrent(mEGLDisplay, mEGLContext);



    mEventControlThread = new EventControlThread(this);

    mEventControlThread->run("EventControl", PRIORITY_URGENT_DISPLAY);
@@ -714,8 +714,11 @@ status_t SurfaceFlinger::getDisplayConfigs(const sp<IBinder>& display, 
            info.density = density;



            // TODO: this needs to go away (currently needed only by webkit)

            sp<const DisplayDevice> hw(getDefaultDisplayDevice());

            {

                Mutex::Autolock _l(mStateLock);

                sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());

                info.orientation = hw->getOrientation();

            }

        } else {

            // TODO: where should this value come from?

            static const int TV_DENSITY = 213;
@@ -772,10 +775,13 @@ int SurfaceFlinger::getActiveConfig(const sp<IBinder>& display) { 
        ALOGE("%s : display is NULL", __func__);

        return BAD_VALUE;

    }



    Mutex::Autolock _l(mStateLock);

    sp<DisplayDevice> device(getDisplayDevice(display));

    if (device != NULL) {

        return device->getActiveConfig();

    }



    return BAD_VALUE;

}
@@ -863,6 +869,7 @@ status_t SurfaceFlinger::getDisplayColorModes(const sp<IBinder>& display, 
}



android_color_mode_t SurfaceFlinger::getActiveColorMode(const sp<IBinder>& display) {

    Mutex::Autolock _l(mStateLock);

    sp<DisplayDevice> device(getDisplayDevice(display));

    if (device != nullptr) {

        return device->getActiveColorMode();
@@ -1124,34 +1131,22 @@ void SurfaceFlinger::getCompositorTiming(CompositorTiming* compositorTiming) { 
    *compositorTiming = mCompositorTiming;

}



void SurfaceFlinger::onHotplugReceived(int32_t disp, bool connected) {

    ALOGV("onHotplugReceived(%d, %s)", disp, connected ? "true" : "false");

    if (disp == DisplayDevice::DISPLAY_PRIMARY) {

        Mutex::Autolock lock(mStateLock);

void SurfaceFlinger::createDefaultDisplayDevice() {

    const int32_t type = DisplayDevice::DISPLAY_PRIMARY;

    wp<IBinder> token = mBuiltinDisplays[type];



    // All non-virtual displays are currently considered secure.

        bool isSecure = true;



        int32_t type = DisplayDevice::DISPLAY_PRIMARY;



        // When we're using the vr composer, the assumption is that we've

        // already created the IBinder object for the primary display.

        if (!mHwc->isUsingVrComposer()) {

            createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);

        }



        wp<IBinder> token = mBuiltinDisplays[type];

    const bool isSecure = true;



    sp<IGraphicBufferProducer> producer;

    sp<IGraphicBufferConsumer> consumer;

        BufferQueue::createBufferQueue(&producer, &consumer,

                new GraphicBufferAlloc());

    BufferQueue::createBufferQueue(&producer, &consumer, new GraphicBufferAlloc());



        sp<FramebufferSurface> fbs = new FramebufferSurface(*mHwc,

                DisplayDevice::DISPLAY_PRIMARY, consumer);

    sp<FramebufferSurface> fbs = new FramebufferSurface(*mHwc, type, consumer);



    bool hasWideColorModes = false;

        std::vector<android_color_mode_t> modes = getHwComposer().getColorModes(type);

    std::vector<android_color_mode_t> modes = getHwComposer().getColorModes(

        type);

    for (android_color_mode_t colorMode : modes) {

        switch (colorMode) {

            case HAL_COLOR_MODE_DISPLAY_P3:
@@ -1163,9 +1158,8 @@ void SurfaceFlinger::onHotplugReceived(int32_t disp, bool connected) { 
                break;

        }

    }

        sp<DisplayDevice> hw =

                new DisplayDevice(this, DisplayDevice::DISPLAY_PRIMARY, disp, isSecure, token, fbs,

                                  producer, mRenderEngine->getEGLConfig(),

    sp<DisplayDevice> hw = new DisplayDevice(this, DisplayDevice::DISPLAY_PRIMARY, type, isSecure,

                                             token, fbs, producer, mRenderEngine->getEGLConfig(),

                                             hasWideColorModes && hasWideColorDisplay);

    mDisplays.add(token, hw);

    android_color_mode defaultColorMode = HAL_COLOR_MODE_NATIVE;
@@ -1173,6 +1167,24 @@ void SurfaceFlinger::onHotplugReceived(int32_t disp, bool connected) { 
        defaultColorMode = HAL_COLOR_MODE_SRGB;

    }

    setActiveColorModeInternal(hw, defaultColorMode);

}



void SurfaceFlinger::onHotplugReceived(HWComposer* composer, int32_t disp, bool connected) {

    ALOGV("onHotplugReceived(%d, %s)", disp, connected ? "true" : "false");



    if (composer->isUsingVrComposer()) {

        // We handle initializing the primary display device for the VR

        // window manager hwc explicitly at the time of transition.

        if (disp != DisplayDevice::DISPLAY_PRIMARY) {

            ALOGE("External displays are not supported by the vr hardware composer.");

        }

        return;

    }



    if (disp == DisplayDevice::DISPLAY_PRIMARY) {

        Mutex::Autolock lock(mStateLock);

        createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);

        createDefaultDisplayDevice();

    } else {

        auto type = DisplayDevice::DISPLAY_EXTERNAL;

        Mutex::Autolock _l(mStateLock);
@@ -1214,6 +1226,7 @@ void SurfaceFlinger::clearHwcLayers(const LayerVector& layers) { 
    }

}



// Note: it is assumed the caller holds |mStateLock| when this is called

void SurfaceFlinger::resetHwc() {

    disableHardwareVsync(true);

    clearHwcLayers(mDrawingState.layersSortedByZ);
@@ -1234,12 +1247,16 @@ void SurfaceFlinger::updateVrFlinger() { 
    if (vrFlingerRequestsDisplay == mHwc->isUsingVrComposer()) {

        return;

    }



    bool vrHwcNewlyInitialized = false;



    if (vrFlingerRequestsDisplay && !mVrHwc) {

        // Construct new HWComposer without holding any locks.

        mVrHwc = new HWComposer(true);

        vrHwcNewlyInitialized = true;

        ALOGV("Vr HWC created");

    }

    {



    Mutex::Autolock _l(mStateLock);



    if (vrFlingerRequestsDisplay) {
@@ -1247,6 +1264,11 @@ void SurfaceFlinger::updateVrFlinger() { 


        mHwc = mVrHwc;

        mVrFlinger->GrantDisplayOwnership();



        if (vrHwcNewlyInitialized) {

            mVrHwc->setEventHandler(

                static_cast<HWComposer::EventHandler*>(this));

        }

    } else {

        mVrFlinger->SeizeDisplayOwnership();
@@ -1258,13 +1280,14 @@ void SurfaceFlinger::updateVrFlinger() { 


    mVisibleRegionsDirty = true;

    invalidateHwcGeometry();



    // Explicitly re-initialize the primary display. This is because some other

    // parts of this class rely on the primary display always being available.

    createDefaultDisplayDevice();



    android_atomic_or(1, &mRepaintEverything);

    setTransactionFlags(eDisplayTransactionNeeded);

}

    if (mVrHwc) {

        mVrHwc->setEventHandler(static_cast<HWComposer::EventHandler*>(this));

    }

}



void SurfaceFlinger::onMessageReceived(int32_t what) {

    ATRACE_CALL();
@@ -1473,7 +1496,8 @@ void SurfaceFlinger::postComposition(nsecs_t refreshStartTime) 
        layer->releasePendingBuffer(dequeueReadyTime);

    }



    const sp<const DisplayDevice> hw(getDefaultDisplayDevice());

    // |mStateLock| not needed as we are on the main thread

    const sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());



    std::shared_ptr<FenceTime> glCompositionDoneFenceTime;

    if (mHwc->hasClientComposition(HWC_DISPLAY_PRIMARY)) {
@@ -1821,7 +1845,8 @@ void SurfaceFlinger::postFramebuffer() 
    mLastSwapBufferTime = systemTime() - now;

    mDebugInSwapBuffers = 0;



    uint32_t flipCount = getDefaultDisplayDevice()->getPageFlipCount();

    // |mStateLock| not needed as we are on the main thread

    uint32_t flipCount = getDefaultDisplayDeviceLocked()->getPageFlipCount();

    if (flipCount % LOG_FRAME_STATS_PERIOD == 0) {

        logFrameStats();

    }
@@ -1906,7 +1931,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) 
                        // Call makeCurrent() on the primary display so we can

                        // be sure that nothing associated with this display

                        // is current.

                        const sp<const DisplayDevice> defaultDisplay(getDefaultDisplayDevice());

                        const sp<const DisplayDevice> defaultDisplay(getDefaultDisplayDeviceLocked());

                        defaultDisplay->makeCurrent(mEGLDisplay, mEGLContext);

                        sp<DisplayDevice> hw(getDisplayDevice(draw.keyAt(i)));

                        if (hw != NULL)
@@ -2096,7 +2121,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) 
                // could be null when this layer is using a layerStack

                // that is not visible on any display. Also can occur at

                // screen off/on times.

                disp = getDefaultDisplayDevice();

                disp = getDefaultDisplayDeviceLocked();

            }

            layer->updateTransformHint(disp);
@@ -2452,7 +2477,9 @@ bool SurfaceFlinger::doComposeSurfaces( 
            ALOGW("DisplayDevice::makeCurrent failed. Aborting surface composition for display %s",

                  displayDevice->getDisplayName().string());

            eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);

            if(!getDefaultDisplayDevice()->makeCurrent(mEGLDisplay, mEGLContext)) {



            // |mStateLock| not needed as we are on the main thread

            if(!getDefaultDisplayDeviceLocked()->makeCurrent(mEGLDisplay, mEGLContext)) {

              ALOGE("DisplayDevice::makeCurrent on default display failed. Aborting.");

            }

            return false;
@@ -3539,7 +3566,7 @@ void SurfaceFlinger::dumpAllLocked(const Vector<String16>& args, size_t& index, 
    colorizer.reset(result);



    HWComposer& hwc(getHwComposer());

    sp<const DisplayDevice> hw(getDefaultDisplayDevice());

    sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());



    colorizer.bold(result);

    result.appendFormat("EGL implementation : %s\n",
@@ -3769,7 +3796,7 @@ status_t SurfaceFlinger::onTransact( 
                return NO_ERROR;

            case 1013: {

                Mutex::Autolock _l(mStateLock);

                sp<const DisplayDevice> hw(getDefaultDisplayDevice());

                sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());

                reply->writeInt32(hw->getPageFlipCount());

                return NO_ERROR;

            }

services/surfaceflinger/SurfaceFlinger.h

+10 −3
Original line number Diff line number Diff line
@@ -184,8 +184,9 @@ public: 
    void repaintEverything();



    // returns the default Display

    sp<const DisplayDevice> getDefaultDisplayDevice() const {

        return getDisplayDevice(mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY]);

    sp<const DisplayDevice> getDefaultDisplayDevice() {

        Mutex::Autolock _l(mStateLock);

        return getDefaultDisplayDeviceLocked();

    }



    // utility function to delete a texture on the main thread
@@ -304,7 +305,7 @@ private: 
     * HWComposer::EventHandler interface

     */

    virtual void onVSyncReceived(HWComposer* composer, int type, nsecs_t timestamp);

    virtual void onHotplugReceived(int disp, bool connected);

    virtual void onHotplugReceived(HWComposer* composer, int disp, bool connected);

    virtual void onInvalidateReceived(HWComposer* composer);



    /* ------------------------------------------------------------------------
@@ -439,6 +440,12 @@ private: 
        return mDisplays.valueFor(dpy);

    }



    sp<const DisplayDevice> getDefaultDisplayDeviceLocked() const {

        return getDisplayDevice(mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY]);

    }



    void createDefaultDisplayDevice();



    int32_t getDisplayType(const sp<IBinder>& display) {

        if (!display.get()) return NAME_NOT_FOUND;

        for (int i = 0; i < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES; ++i) {