Fix race conditions when using mHwc off the main thread (1f3e4bbb) · Commits · e / os / android_frameworks_native-old

services/surfaceflinger/SurfaceFlinger.cpp

+23 −5

Original line number	Diff line number	Diff line
		@@ -632,6 +632,8 @@ void SurfaceFlinger::init() {
		auto vrFlingerRequestDisplayCallback = [this] (bool requestDisplay) {
		ALOGI("VR request display mode: requestDisplay=%d", requestDisplay);
		mVrFlingerRequestsDisplay = requestDisplay;
		ConditionalLock _l(mStateLock,
		std::this_thread::get_id() != mMainThreadId);
		signalTransaction();
		};
		mVrFlinger = dvr::VrFlinger::Create(mHwc->getComposer(),
		@@ -719,6 +721,8 @@ status_t SurfaceFlinger::getSupportedFrameTimestamps(
		FrameEvent::DEQUEUE_READY,
		FrameEvent::RELEASE,
		};
		ConditionalLock _l(mStateLock,
		std::this_thread::get_id() != mMainThreadId);
		if (!getHwComposer().hasCapability(
		HWC2::Capability::PresentFenceIsNotReliable)) {
		outSupported->push_back(FrameEvent::DISPLAY_PRESENT);
		@@ -766,6 +770,8 @@ status_t SurfaceFlinger::getDisplayConfigs(const sp<IBinder>& display,

		configs->clear();

		ConditionalLock _l(mStateLock,
		std::this_thread::get_id() != mMainThreadId);
		for (const auto& hwConfig : getHwComposer().getConfigs(type)) {
		DisplayInfo info = DisplayInfo();

		@@ -789,7 +795,7 @@ 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());
		sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
		info.orientation = hw->getOrientation();
		} else {
		// TODO: where should this value come from?
		@@ -932,7 +938,12 @@ status_t SurfaceFlinger::getDisplayColorModes(const sp<IBinder>& display,
		return type;
		}

		std::vector<android_color_mode_t> modes = getHwComposer().getColorModes(type);
		std::vector<android_color_mode_t> modes;
		{
		ConditionalLock _l(mStateLock,
		std::this_thread::get_id() != mMainThreadId);
		modes = getHwComposer().getColorModes(type);
		}
		outColorModes->clear();
		std::copy(modes.cbegin(), modes.cend(), std::back_inserter(*outColorModes));

		@@ -1340,7 +1351,7 @@ void SurfaceFlinger::onRefreshReceived(int sequenceId,
		if (sequenceId != mComposerSequenceId) {
		return;
		}
		repaintEverything();
		repaintEverythingLocked();
		}

		void SurfaceFlinger::setVsyncEnabled(int disp, int enabled) {
		@@ -3348,7 +3359,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& hw,

		mVisibleRegionsDirty = true;
		mHasPoweredOff = true;
		repaintEverything();
		repaintEverythingLocked();

		struct sched_param param = {0};
		param.sched_priority = 1;
		@@ -3992,6 +4003,7 @@ status_t SurfaceFlinger::onTransact(
		return NO_ERROR;
		}
		case 1005:{ // force transaction
		Mutex::Autolock _l(mStateLock);
		setTransactionFlags(
		eTransactionNeeded\|
		eDisplayTransactionNeeded\|
		@@ -4128,11 +4140,17 @@ status_t SurfaceFlinger::onTransact(
		return err;
		}

		void SurfaceFlinger::repaintEverything() {
		void SurfaceFlinger::repaintEverythingLocked() {
		android_atomic_or(1, &mRepaintEverything);
		signalTransaction();
		}

		void SurfaceFlinger::repaintEverything() {
		ConditionalLock _l(mStateLock,
		std::this_thread::get_id() != mMainThreadId);
		repaintEverythingLocked();
		}

		// Checks that the requested width and height are valid and updates them to the display dimensions
		// if they are set to 0
		static status_t updateDimensionsLocked(const sp<const DisplayDevice>& displayDevice,

services/surfaceflinger/SurfaceFlinger.h

+25 −2

Original line number	Diff line number	Diff line
		@@ -192,6 +192,8 @@ public:

		// force full composition on all displays
		void repaintEverything();
		// Can only be called from the main thread or with mStateLock held
		void repaintEverythingLocked();

		// returns the default Display
		sp<const DisplayDevice> getDefaultDisplayDevice() const {
		@@ -344,7 +346,9 @@ private:
		* Message handling
		*/
		void waitForEvent();
		// Can only be called from the main thread or with mStateLock held
		void signalTransaction();
		// Can only be called from the main thread or with mStateLock held
		void signalLayerUpdate();
		void signalRefresh();

		@@ -387,6 +391,7 @@ private:
		*/
		uint32_t getTransactionFlags(uint32_t flags);
		uint32_t peekTransactionFlags();
		// Can only be called from the main thread or with mStateLock held
		uint32_t setTransactionFlags(uint32_t flags);
		void commitTransaction();
		uint32_t setClientStateLocked(const sp<Client>& client, const layer_state_t& s);
		@@ -643,8 +648,26 @@ private:
		// access must be protected by mInvalidateLock
		volatile int32_t mRepaintEverything;

		// The current hardware composer interface. When switching into and out of
		// vr, our HWComposer instance will be recreated.
		// The current hardware composer interface.
		//
		// The following thread safety rules apply when accessing mHwc, either
		// directly or via getHwComposer():
		//
		// 1. When recreating mHwc, acquire mStateLock. We currently recreate mHwc
		// only when switching into and out of vr. Recreating mHwc must only be
		// done on the main thread.
		//
		// 2. When accessing mHwc on the main thread, it's not necessary to acquire
		// mStateLock.
		//
		// 3. When accessing mHwc on a thread other than the main thread, we always
		// need to acquire mStateLock. This is because the main thread could be
		// in the process of destroying the current mHwc instance.
		//
		// The above thread safety rules only apply to SurfaceFlinger.cpp. In
		// SurfaceFlinger_hwc1.cpp we create mHwc at surface flinger init and never
		// destroy it, so it's always safe to access mHwc from any thread without
		// acquiring mStateLock.
		std::unique_ptr<HWComposer> mHwc;

		#ifdef USE_HWC2