crash in mozilla::image::ProgressTracker::SyncNotify(imgRequestProxy*)

ASSIGNED
Assigned to

Status

()

Core
ImageLib
--
critical
ASSIGNED
3 years ago
2 years ago

People

(Reporter: Petruta Rasa [Away. Please needinfo? bogdan.maris@softvision.ro], Assigned: aosmond)

Tracking

({crash})

36 Branch
All
Windows 8.1
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-648e5ee7-6922-457e-8a52-a19da2150203.
=============================================================

Reproduced on loading http://download.wavetlan.com/SVV/Media/HTTP/MP4/ConvertedFiles/QuickTime/QuickTime_test10_3m2s_AVC_VBR_762kbps_176x144_25fps_AAC-LCv4_CBR_320kbps_Stereo_48000Hz.mp4 - slow buffering on this site, so it could also be a networking issue. (choosing Video/Audio component for now)
I had other tabs opened containing mp4 files, several of them where loading slowly. 

Unfortunately I couldn't reproduce the crash again, so I have no reliable steps.
Component: Audio/Video → Audio/Video: Playback

Updated

3 years ago
Crash Signature: [@ mozilla::image::ProgressTracker::SyncNotify(imgRequestProxy*)] → [@ mozilla::image::ProgressTracker::SyncNotify(imgRequestProxy*)] [@ mozilla::image::ProgressTracker::SyncNotify]
There are only 2 crash reports in the last 28 days both of which have been Firefox 36. I'm going to close this bug. Re-open it if it is still an issue.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 2

2 years ago
This appears to have resurfaced on more recent builds:

https://crash-stats.mozilla.com/report/index/4ec21904-fe46-44a4-b1a5-b12c22160928
https://crash-stats.mozilla.com/report/index/4246964c-8248-440b-a59a-4bc3c2161002

It seems as if the underlying image got freed as it sometimes fails the VMT lookup.
Assignee: nobody → aosmond
Status: RESOLVED → REOPENED
Component: Audio/Video: Playback → ImageLib
Resolution: FIXED → ---
(Assignee)

Updated

2 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 3

2 years ago
Created attachment 8798534 [details] [diff] [review]
Remove mozilla::image::ProgressTracker's stored raw Image pointer, v1

The conflict this patch tries to avoid is where:
1) RasterImage is getting freed on one thread, its destructor is called and blocks on the mutex lock in ProgressTracker::ResetImage.
2) Another thread holds said mutex and acquires a strong reference to RasterImage, but it is going to be destroyed.
3) Later on it tries to use that reference, but the memory it was stored in either got cleared (hence the 0xe5e5e5e5 pointers observed) and/or reused (hence why some stack traces wander before crashing or continue slightly further on).

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4833065731ef9e43a0bf5ebcac23992e52ced2f

Ideally we could reduce eliminate the need for Image in ProgressTracker completely because it only uses it for 1) getting the URI, 2) getting the image dimensions in ProgressTracker::SyncNotify and 3) knowing that we have an image at all.

For 1), a strong reference to that object can easily be stored at ProgressTracker construction time.

For 2), the dimensions are only used for an empty/non-empty check. If Image::mError is true, GetWidth/GetHeight will fail, and it will also set the dimensions to non-empty. Thus the only time it is empty is before we get the size dimension.

For 3), I wonder if it is possible that not holding onto the image itself (in one of the async runnables) could potentially change the state machine in a minor/subtle way.
You need to log in before you can comment on or make changes to this bug.