stylo: Unsafe parallel refcount drop in CachedBorderImageData::PurgeCachedImages

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: bholley, Unassigned)

Tracking

unspecified
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

> > Error: AddRef/Release on nsISupports
> > Location: nsCOMArray.cpp:void ReleaseObjects(class nsTArray<nsISupports*>*)
> > @ xpcom/glue/nsCOMArray.cpp:272
> > Stack Trace:
> > void nsCOMArray_base::Clear() @ xpcom/glue/nsCOMArray.cpp:281
> > void CachedBorderImageData::PurgeCachedImages() @
> > layout/style/nsStyleStruct.cpp:2044
> > void nsStyleImage::SetImageRequest(struct
> > already_AddRefed<nsStyleImageRequest>) @ layout/style/nsStyleStruct.cpp:2150
> > Gecko_SetUrlImageValue @ layout/style/ServoBindings.cpp:752
Manish, you were working on this code, right? WDYT?
Flags: needinfo?(manishearth)
I think heycam will have a better idea -- he did the final implementation of the url image stuff.
Flags: needinfo?(manishearth) → needinfo?(cam)
Some naive options:
* Check IsInServoTraversal in PurgeCachedImages, and NS_ReleaseOnMainThread if so.
* Queue up this work (on the servo side) in SequentialTasks to execute post-traversal.
Flags: needinfo?(cam)
Comment on attachment 8839141 [details]
Bug 1335321 - stylo: Do CachedBorderImageData::PurgeCachedImages work on the main thread.

https://reviewboard.mozilla.org/r/113864/#review115456

::: layout/style/nsStyleStruct.cpp:2074
(Diff revision 1)
>  CachedBorderImageData::PurgeCachedImages()
>  {
> +  if (ServoStyleSet::IsInServoTraversal()) {
> +    RefPtr<PurgeCachedImagesTask> task = new PurgeCachedImagesTask();
> +    task->mSubImages.SwapElements(mSubImages);
> +    NS_DispatchToMainThread(task.forget());

Maybe add a comment saying that this won't proxy if we're already on the main thread, and that's fine.
Attachment #8839141 - Flags: review?(bobbyholley) → review+
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b393d50b5ed8
stylo: Do CachedBorderImageData::PurgeCachedImages work on the main thread. r=bholley
https://hg.mozilla.org/mozilla-central/rev/b393d50b5ed8
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.