Closed Bug 976924 Opened 11 years ago Closed 10 years ago

Intermittent test_bug852315.html | OnDestroy callback ran and did not crash

Categories

(Core Graveyard :: Plug-ins, defect)

Product:
Core Graveyard
Component:
Plug-ins
Version:
29 Branch
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox33 wontfix, firefox34 wontfix, firefox35 wontfix, firefox36 wontfix, firefox38 wontfix, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed, firefox-esr24 unaffected, firefox-esr31 wontfix, firefox-esr38 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox33 --- wontfix
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- wontfix
firefox38 --- wontfix
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed
firefox-esr24 --- unaffected
firefox-esr31 --- wontfix
firefox-esr38 --- fixed

People

(Reporter: KWierso, Assigned: bugzilla)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Ensure we're not racing with async plugin teardown events
1.05 KB, patch
gfritzsche
: review+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=35249357&tree=Mozilla-Inbound slave: talos-r4-snow-101 17:33:54 INFO - 59 INFO TEST-INFO | MEMORY STAT residentFast after test: 208556032 17:33:54 INFO - 60 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 57186288 17:33:54 INFO - 61 INFO TEST-END | /tests/dom/plugins/test/mochitest/test_bug813906.html | finished in 181ms 17:33:54 INFO - 62 INFO TEST-START | /tests/dom/plugins/test/mochitest/test_bug852315.html 17:33:54 INFO - 63 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/plugins/test/mochitest/test_bug852315.html | OnDestroy callback ran and did not crash 17:33:54 INFO - 64 INFO TEST-INFO | MEMORY STAT vsize after test: 3314348032 17:33:54 INFO - 65 INFO TEST-INFO | MEMORY STAT residentFast after test: 208572416 17:33:54 INFO - 66 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 58575136 17:33:54 INFO - NPP_Destroy 17:33:54 INFO - 67 INFO TEST-END | /tests/dom/plugins/test/mochitest/test_bug852315.html | finished in 81ms 17:33:54 INFO - 68 INFO TEST-START | /tests/dom/plugins/test/mochitest/test_bug854082.html 17:33:54 INFO - 69 INFO TEST-INFO | MEMORY STAT vsize after test: 3314413568 17:33:54 INFO - 70 INFO TEST-INFO | MEMORY STAT residentFast after test: 208617472 17:33:54 INFO - 71 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 58783400
This seems likely to be from John's recent landings given the timing.
Flags: needinfo?(jschoenick)
Yeah, this test is assuming one event loop is all it will take to despawn the plugin, but that could be racey. This test can probably just trigger from callOnDestroy directly.
Status: NEW → ASSIGNED
Flags: needinfo?(jschoenick)
Assignee: nobody → jschoenick
Attachment #8382654 - Flags: review?(georg.fritzsche)
Comment on attachment 8382654 [details] [diff] [review] Ensure we're not racing with async plugin teardown events Review of attachment 8382654 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure if one more executeSoon makes it completely deterministic, but it certainly can't make the situation worse.
Attachment #8382654 - Flags: review?(georg.fritzsche) → review+
(In reply to Georg Fritzsche [:gfritzsche] from comment #16) > Comment on attachment 8382654 [details] [diff] [review] > Ensure we're not racing with async plugin teardown events > > Review of attachment 8382654 [details] [diff] [review]: > ----------------------------------------------------------------- > > I'm not sure if one more executeSoon makes it completely deterministic, but > it certainly can't make the situation worse. I think it should, anything queued by navigating the page should occur before the first executeSoon event, and thus any CheckPluginStopEvents before the second -- but I'll admit I don't know why it's not deterministic as-is. Let's see if TBPL proves me wrong. https://hg.mozilla.org/integration/mozilla-inbound/rev/996d9d1af045
https://hg.mozilla.org/mozilla-central/rev/996d9d1af045
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This appears to be consistently reproducible when run on our experimental aws Windows Server 2012 test machines.
Blocks: win32test
I was able to make this run green on my AWS Server 2012 VM by changing executeSoon to a setTimeout(0). I assume this is because executeSoon uses Components.utils.dispatch if SpecialPowers are present. Does this make any sense?
Flags: needinfo?(jschoenick)
(In reply to Aaron Klotz [:aklotz] from comment #26) > I was able to make this run green on my AWS Server 2012 VM by changing > executeSoon to a setTimeout(0). I assume this is because executeSoon uses > Components.utils.dispatch if SpecialPowers are present. Does this make any > sense? I'm not familiar the effective differences -- maybe? After removing the plugin, we should either have destroyed it or queued an event to destroy it, so a event queued after *that* should be late enough. But, we queue plugin teardown checks with RunInStableState(), so if utils.dispatch events can occur before stablestate that would explain it.
Flags: needinfo?(jschoenick)
No longer blocks: win32test
Andrew, can you suggest an owner for this with John gone?
Assignee: john → nobody
status-firefox28: unaffected → ---
status-firefox29: unaffected → ---
status-firefox30: fixed → ---
status-firefox33: --- → wontfix
status-firefox34: --- → wontfix
status-firefox35: --- → affected
status-firefox36: --- → affected
status-firefox-esr31: --- → affected
Flags: needinfo?(overholt)
Target Milestone: mozilla30 → ---
Bill's my best guess.
Flags: needinfo?(overholt) → needinfo?(wmccloskey)
At this point I think Aaron is the best person to look at this bug since he can reproduce it and he understands plugins.
Flags: needinfo?(wmccloskey) → needinfo?(aklotz)
I think this is probably going to require some mitigations similar to the ones that I'm working on in bug 1158761.
Flags: needinfo?(aklotz)
I beefed up this test as part of my fixes in bug 1158761, so I'm going to resolve this.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
Flags: qe-verify-
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: