Disable the crash guard in the GPU process

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

3 years ago
This hasn't been made to work yet (it's disabled in Nightly builds so hasn't caused problems yet) and is basically obseleted when we have the GPU process.

Let's just disable it for now.
Attachment #8808898 - Flags: review?(dvander)
Assignee

Comment 1

3 years ago
If the crash guard isn't enabled, then we don't need to sync dispatch to the main thread to initialize DXVA.

This fixes a bug that tryserver found, where NS_DISPATCH_SYNC spins the event loop of the current thread while it waits, and we end up trying to handle Shutdown() (on VideoDecoderParent) while we're still in the middle of Init.
Attachment #8808899 - Flags: review?(jyavenard)
Attachment #8808898 - Flags: review?(dvander) → review+
Comment on attachment 8808899 [details] [diff] [review]
dont-sync-dispatch-gpu-process

Review of attachment 8808899 [details] [diff] [review]:
-----------------------------------------------------------------

I can understand that we don't need to do anything on the main thread (as that the crashguard is the only reason we need to run on the main thread anyway).

But why not use SyncRunnable instead? this doesn't start an event loop.

Otherwise r=me with the telemetry stuff put back

::: dom/media/platforms/wmf/WMFVideoMFTManager.cpp
@@ +125,5 @@
>                         mGotValidOutputAfterNullOutput ? 3 :
>                         4;
>  
> +  LOG(nsPrintfCString("Reporting telemetry VIDEO_MFT_OUTPUT_NULL_SAMPLES=%d", telemetry).get());
> +  Telemetry::Accumulate(Telemetry::ID::VIDEO_MFT_OUTPUT_NULL_SAMPLES, telemetry);

this seems unrelated... this isn't a sync dispatch.

And I believe telemetry needs to be run on the main thread regardless.
Attachment #8808899 - Flags: review?(jyavenard) → review+
Assignee

Comment 3

3 years ago
Posted patch sync-runnableSplinter Review
Trying again using SyncRunnable.

Unfortunately the DXVA blacklist code uses ClearOnShutdown which is main thread only, so the previous patch had issues on try.
Attachment #8808899 - Attachment is obsolete: true
Attachment #8809199 - Flags: review?(jyavenard)
Attachment #8809199 - Flags: review?(jyavenard) → review+

Comment 4

3 years ago
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccc2831433b6
Use SyncRunnable instead of NS_DISPATCH_SYNC in WMFVideoMFTManager since the latter can spin the event loop. r=jya
https://hg.mozilla.org/integration/mozilla-inbound/rev/f210f6b4c5cd
Disable the crash guard in the GPU process since it won't be useful. r=dvander

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ccc2831433b6
https://hg.mozilla.org/mozilla-central/rev/f210f6b4c5cd
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.