Closed Bug 1265619 Opened 4 years ago Closed 4 years ago

Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.5+, b2g-v2.5 fixed, b2g-master unaffected)

RESOLVED FIXED
blocking-b2g 2.5+
Tracking Status
b2g-v2.5 --- fixed
b2g-master --- unaffected

People

(Reporter: kikuo, Assigned: kikuo)

References

Details

(Whiteboard: [ft:conndevices])

Attachments

(1 file)

According to Bug 1262369 Comment 21, APZCTreeManager::InitializeLayerForId() [1] is dispatched to main thread for execution, and in the meanwhile APZCTreeManager::PrepareNodeForLayer() [2] may be executed before the initialization is done and will lead to a crash at mPaintThrottler->TaskComplete(GetFrameTime()) [3].

[1] https://github.com/mozilla/gecko-dev/blob/b2g44_v2_5/gfx/layers/apz/src/APZCTreeManager.cpp#L212
[2] https://github.com/mozilla/gecko-dev/blob/b2g44_v2_5/gfx/layers/apz/src/APZCTreeManager.cpp#L373
[3] https://github.com/mozilla/gecko-dev/blob/b2g44_v2_5/gfx/layers/apz/src/AsyncPanZoomController.cpp#L3083

This won't happen on m-c now, I will fix this on b2g44 based on Bug 1262369 Comment 24.
 - Directly initializing mPaintThrottlerMap without dispatching the task to main thread
 - Introducing a mutex to protect the mPaintThrottlerMap.
Assignee: nobody → kikuo
[Blocking Requested - why for this release]:
May lead to crash, to fix for better quality.
blocking-b2g: --- → 2.5?
blocking-b2g: 2.5? → 2.5+
Whiteboard: [ft:conndevices]
Attachment #8742691 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8742691 [details]
[b2g44_v25] MozReview Request: Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats.

https://reviewboard.mozilla.org/r/47423/#review44151

This looks fine, thanks! Please make sure to land directly on b2g44 (obviously it won't apply on master).
Attachment #8742691 - Attachment description: MozReview Request: Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats. → [b2g44_v25] MozReview Request: Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats.
Keywords: checkin-needed
Comment on attachment 8742691 [details]
[b2g44_v25] MozReview Request: Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Timing issue.
User impact if declined: App MAY crash if there's no protection for the map data.
Testing completed: On nexus-player, Partner's PF, all good. https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfc5663d7653
Risk to taking this patch (and alternatives if risky): Add a mutex to guarantee only one can access at a time, risk should be low.
String or UUID changes made by this patch: None
Attachment #8742691 - Flags: approval‑mozilla‑b2g44?
Keywords: checkin-needed
Comment on attachment 8742691 [details]
[b2g44_v25] MozReview Request: Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats.

Approve for TV 2.5
Attachment #8742691 - Flags: approval‑mozilla‑b2g44? → approval‑mozilla‑b2g44+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Blocks: TV_FxOS2.5
You need to log in before you can comment on or make changes to this bug.