Skip to content

Commit 67346b9

Browse files
author
Ben Widawsky
committed
SF: Annotate more lock requirements
Upcoming reworks will require more use of mStateLock and having these in place helps avoid footguns. Test: builds Flag: EXEMPT refactor Change-Id: I37439673e1cdb67df5ca3dd524c9a72d34abc497
1 parent adcbb98 commit 67346b9

2 files changed

Lines changed: 32 additions & 25 deletions

File tree

‎services/surfaceflinger/Layer.cpp‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ aidl::android::hardware::graphics::composer3::Composition Layer::getCompositionT
362362
// transaction
363363
// ----------------------------------------------------------------------------
364364

365-
void Layer::commitTransaction() {
365+
void Layer::commitTransaction() REQUIRES(mFlinger->mStateLock) {
366366
// Set the present state for all bufferlessSurfaceFramesTX to Presented. The
367367
// bufferSurfaceFrameTX will be presented in latchBuffer.
368368
for (auto& [token, surfaceFrame] : mDrawingState.bufferlessSurfaceFramesTX) {
@@ -394,7 +394,8 @@ bool Layer::isLayerFocusedBasedOnPriority(int32_t priority) {
394394
};
395395

396396
void Layer::setFrameTimelineVsyncForBufferTransaction(const FrameTimelineInfo& info,
397-
nsecs_t postTime, gui::GameMode gameMode) {
397+
nsecs_t postTime, gui::GameMode gameMode)
398+
REQUIRES(mFlinger->mStateLock) {
398399
mDrawingState.postTime = postTime;
399400

400401
// Check if one of the bufferlessSurfaceFramesTX contains the same vsyncId. This can happen if
@@ -458,15 +459,16 @@ void Layer::addSurfaceFrameDroppedForBuffer(
458459

459460
void Layer::addSurfaceFramePresentedForBuffer(
460461
std::shared_ptr<frametimeline::SurfaceFrame>& surfaceFrame, nsecs_t acquireFenceTime,
461-
nsecs_t currentLatchTime) {
462+
nsecs_t currentLatchTime) REQUIRES(mFlinger->mStateLock) {
462463
surfaceFrame->setAcquireFenceTime(acquireFenceTime);
463464
surfaceFrame->setPresentState(PresentState::Presented, mLastLatchTime);
464465
mFlinger->mFrameTimeline->addSurfaceFrame(surfaceFrame);
465466
updateLastLatchTime(currentLatchTime);
466467
}
467468

468469
std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForTransaction(
469-
const FrameTimelineInfo& info, nsecs_t postTime, gui::GameMode gameMode) {
470+
const FrameTimelineInfo& info, nsecs_t postTime, gui::GameMode gameMode)
471+
REQUIRES(mFlinger->mStateLock) {
470472
auto surfaceFrame =
471473
mFlinger->mFrameTimeline->createSurfaceFrameForToken(info, mOwnerPid, mOwnerUid,
472474
getSequence(), mName,
@@ -488,7 +490,7 @@ std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForTransac
488490

489491
std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForBuffer(
490492
const FrameTimelineInfo& info, nsecs_t queueTime, std::string debugName,
491-
gui::GameMode gameMode) {
493+
gui::GameMode gameMode) REQUIRES(mFlinger->mStateLock) {
492494
auto surfaceFrame =
493495
mFlinger->mFrameTimeline->createSurfaceFrameForToken(info, mOwnerPid, mOwnerUid,
494496
getSequence(), mName, debugName,
@@ -506,7 +508,8 @@ std::shared_ptr<frametimeline::SurfaceFrame> Layer::createSurfaceFrameForBuffer(
506508
}
507509

508510
void Layer::setFrameTimelineVsyncForSkippedFrames(const FrameTimelineInfo& info, nsecs_t postTime,
509-
std::string debugName, gui::GameMode gameMode) {
511+
std::string debugName, gui::GameMode gameMode)
512+
REQUIRES(mFlinger->mStateLock) {
510513
if (info.skippedFrameVsyncId == FrameTimelineInfo::INVALID_VSYNC_ID) {
511514
return;
512515
}
@@ -842,7 +845,7 @@ bool Layer::setTransformToDisplayInverse(bool transformToDisplayInverse) {
842845
return true;
843846
}
844847

845-
void Layer::releasePreviousBuffer() {
848+
void Layer::releasePreviousBuffer() REQUIRES(mFlinger->mStateLock) {
846849
mReleasePreviousBuffer = true;
847850
if (!mBufferInfo.mBuffer ||
848851
(!mDrawingState.buffer->hasSameBuffer(*mBufferInfo.mBuffer) ||
@@ -884,7 +887,8 @@ void Layer::resetDrawingStateBufferInfo() {
884887

885888
bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer,
886889
const BufferData& bufferData, nsecs_t postTime, nsecs_t desiredPresentTime,
887-
bool isAutoTimestamp, const FrameTimelineInfo& info, gui::GameMode gameMode) {
890+
bool isAutoTimestamp, const FrameTimelineInfo& info, gui::GameMode gameMode)
891+
REQUIRES(mFlinger->mStateLock) {
888892
SFTRACE_FORMAT("setBuffer %s - hasBuffer=%s", getDebugName(), (buffer ? "true" : "false"));
889893

890894
const bool frameNumberChanged =
@@ -1074,7 +1078,8 @@ bool Layer::setDesiredHdrHeadroom(float desiredRatio) {
10741078
}
10751079

10761080
bool Layer::setSidebandStream(const sp<NativeHandle>& sidebandStream, const FrameTimelineInfo& info,
1077-
nsecs_t postTime, gui::GameMode gameMode) {
1081+
nsecs_t postTime, gui::GameMode gameMode)
1082+
REQUIRES(mFlinger->mStateLock) {
10781083
if (mDrawingState.sidebandStream == sidebandStream) return false;
10791084

10801085
if (mDrawingState.sidebandStream != nullptr && sidebandStream == nullptr) {
@@ -1207,7 +1212,7 @@ bool Layer::latchSidebandStream(bool& recomputeVisibleRegions) {
12071212
return false;
12081213
}
12091214

1210-
void Layer::updateTexImage(nsecs_t latchTime, bool bgColorOnly) {
1215+
void Layer::updateTexImage(nsecs_t latchTime, bool bgColorOnly) REQUIRES(mFlinger->mStateLock) {
12111216
const State& s(getDrawingState());
12121217

12131218
if (!s.buffer) {
@@ -1457,7 +1462,8 @@ void Layer::onCompositionPresented(const DisplayDevice* display,
14571462
mBufferInfo.mFrameLatencyNeeded = false;
14581463
}
14591464

1460-
bool Layer::latchBufferImpl(bool& recomputeVisibleRegions, nsecs_t latchTime, bool bgColorOnly) {
1465+
bool Layer::latchBufferImpl(bool& recomputeVisibleRegions, nsecs_t latchTime, bool bgColorOnly)
1466+
REQUIRES(mFlinger->mStateLock) {
14611467
SFTRACE_FORMAT_INSTANT("latchBuffer %s - %" PRIu64, getDebugName(),
14621468
getDrawingState().frameNumber);
14631469

‎services/surfaceflinger/SurfaceFlinger.cpp‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2456,7 +2456,8 @@ void SurfaceFlinger::updateLayerHistory(nsecs_t now) {
24562456
}
24572457

24582458
bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
2459-
bool flushTransactions, bool& outTransactionsAreEmpty) {
2459+
bool flushTransactions, bool& outTransactionsAreEmpty)
2460+
EXCLUDES(mStateLock) {
24602461
using Changes = frontend::RequestedLayerState::Changes;
24612462
SFTRACE_CALL();
24622463
SFTRACE_NAME_FOR_TRACK(WorkloadTracer::TRACK_NAME, "Transaction Handling");
@@ -2653,7 +2654,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
26532654
}
26542655

26552656
bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId,
2656-
const scheduler::FrameTargets& frameTargets) {
2657+
const scheduler::FrameTargets& frameTargets) EXCLUDES(mStateLock) {
26572658
const scheduler::FrameTarget& pacesetterFrameTarget = *frameTargets.get(pacesetterId)->get();
26582659

26592660
const VsyncId vsyncId = pacesetterFrameTarget.vsyncId();
@@ -4847,12 +4848,14 @@ bool SurfaceFlinger::flushTransactionQueues() {
48474848
return applyTransactions(transactions);
48484849
}
48494850

4850-
bool SurfaceFlinger::applyTransactions(std::vector<QueuedTransactionState>& transactions) {
4851+
bool SurfaceFlinger::applyTransactions(std::vector<QueuedTransactionState>& transactions)
4852+
EXCLUDES(mStateLock) {
48514853
Mutex::Autolock lock(mStateLock);
48524854
return applyTransactionsLocked(transactions);
48534855
}
48544856

4855-
bool SurfaceFlinger::applyTransactionsLocked(std::vector<QueuedTransactionState>& transactions) {
4857+
bool SurfaceFlinger::applyTransactionsLocked(std::vector<QueuedTransactionState>& transactions)
4858+
REQUIRES(mStateLock) {
48564859
bool needsTraversal = false;
48574860
// Now apply all transactions.
48584861
for (auto& transaction : transactions) {
@@ -5069,15 +5072,13 @@ status_t SurfaceFlinger::setTransactionState(
50695072
return NO_ERROR;
50705073
}
50715074

5072-
bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelineInfo,
5073-
std::vector<ResolvedComposerState>& states,
5074-
Vector<DisplayState>& displays, uint32_t flags,
5075-
const InputWindowCommands& inputWindowCommands,
5076-
const int64_t desiredPresentTime, bool isAutoTimestamp,
5077-
const std::vector<uint64_t>& uncacheBufferIds,
5078-
const int64_t postTime, bool hasListenerCallbacks,
5079-
const std::vector<ListenerCallbacks>& listenerCallbacks,
5080-
int originPid, int originUid, uint64_t transactionId) {
5075+
bool SurfaceFlinger::applyTransactionState(
5076+
const FrameTimelineInfo& frameTimelineInfo, std::vector<ResolvedComposerState>& states,
5077+
Vector<DisplayState>& displays, uint32_t flags,
5078+
const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime,
5079+
bool isAutoTimestamp, const std::vector<uint64_t>& uncacheBufferIds, const int64_t postTime,
5080+
bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
5081+
int originPid, int originUid, uint64_t transactionId) REQUIRES(mStateLock) {
50815082
uint32_t transactionFlags = 0;
50825083

50835084
// start and end registration for listeners w/ no surface so they can get their callback. Note
@@ -5229,7 +5230,7 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f
52295230
ResolvedComposerState& composerState,
52305231
int64_t desiredPresentTime,
52315232
bool isAutoTimestamp, int64_t postTime,
5232-
uint64_t transactionId) {
5233+
uint64_t transactionId) REQUIRES(mStateLock) {
52335234
layer_state_t& s = composerState.state;
52345235

52355236
std::vector<ListenerCallbacks> filteredListeners;

0 commit comments

Comments
 (0)