Closed Bug 1545575 Opened 5 months ago Closed 5 months ago

Fix potential content process crash from unclean RDD shutdown

Categories

(Core :: Audio/Video: Playback, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: mjf, Assigned: mjf)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

If the linux sandbox fails to initalize properly in RDDChild::Init[1], don't use KillHard[2] to stop the RDD process because it will cause the crash signature from Bug 1535335. This type of sandbox init failure is supposed to be rare, but if it happens, it is a guaranteed content process crash because we get to RemoteVideoDecoderChild::InitIPDL here[3] and the call to SendPRemoteDecoderConstructor crashes the tab if the IPC channel is not valid.

[1] https://searchfox.org/mozilla-central/source/dom/media/ipc/RDDChild.cpp#43
[2] https://searchfox.org/mozilla-central/source/dom/media/ipc/RDDProcessHost.cpp#167
[3] https://searchfox.org/mozilla-central/source/dom/media/ipc/RemoteVideoDecoder.cpp#129

Assignee: nobody → mfroman
Priority: -- → P2

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task
Pushed by mfroman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f78de85608ea
stop RDD process cleanly if linux sandbox fails to init. r=bryce
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Type: task → defect
Blocks: RDD
Depends on: 1515497

Comment on attachment 9060413 [details]
Bug 1545575 - stop RDD process cleanly if linux sandbox fails to init. r?bryce

Beta/Release Uplift Approval Request

  • User impact if declined: If an error occurs while trying to initialize the linux sandbox in the RDD process, it will crash the tab (content process).
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1515497
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): A very small change that causes RDD to shut down nicely instead of killing it abruptly in the case of linux sandbox initialization error.
  • String changes made/needed: n/a
Attachment #9060413 - Flags: approval-mozilla-beta?

Comment on attachment 9060413 [details]
Bug 1545575 - stop RDD process cleanly if linux sandbox fails to init. r?bryce

Minimal patch preventing crashes on Linux, uplift approved for 67 beta 15, thanks.

Attachment #9060413 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hi Michael, uplifted this to beta and got a conflict. Tried to fix it, these are my changes: https://hg.mozilla.org/releases/mozilla-beta/rev/9d654bcc7cae40a6158827030d6ce179ca2392a9#l1.12

Backed out 3 changesets (bug 1515497, bug 1545575) for build bustages at dom/media/ipc/RDDProcessHost.cpp on a CLOSED TREE

Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/9d5221dfb552ab7d00130cebbd66a3b674b1ae2e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&revision=e9bdef8981147f7838c32db18d98062aff05ee0f&selectedJob=243205714

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=243205714&repo=mozilla-beta&lineNumber=17932

Log snippet:

[task 2019-04-28T20:42:58.207Z] 20:42:58 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/bindings'
[task 2019-04-28T20:42:58.811Z] 20:42:58 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/ipc'
[task 2019-04-28T20:42:58.813Z] 20:42:58 INFO - /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o RDDProcessHost.i_o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/media/ipc -I/builds/worker/workspace/build/src/obj-firefox/dom/media/ipc -I/builds/worker/workspace/build/src/xpcom/base -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fomit-frame-pointer -funwind-tables -Werror -fprofile-instr-generate -MD -MP -MF .deps/RDDProcessHost.i_o.pp /builds/worker/workspace/build/src/dom/media/ipc/RDDProcessHost.cpp
[task 2019-04-28T20:42:58.813Z] 20:42:58 ERROR - /builds/worker/workspace/build/src/dom/media/ipc/RDDProcessHost.cpp:139:26: error: too many arguments to function call, expected 0, have 1
[task 2019-04-28T20:42:58.814Z] 20:42:58 INFO - if (!mRDDChild->Init(startMacSandbox)) {
[task 2019-04-28T20:42:58.814Z] 20:42:58 INFO - ~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~
[task 2019-04-28T20:42:58.814Z] 20:42:58 INFO - /builds/worker/workspace/build/src/dom/media/ipc/RDDChild.h:35:3: note: 'Init' declared here
[task 2019-04-28T20:42:58.814Z] 20:42:58 INFO - bool Init();
[task 2019-04-28T20:42:58.814Z] 20:42:58 INFO - ^
[task 2019-04-28T20:42:58.814Z] 20:42:58 INFO - 1 error generated.
[task 2019-04-28T20:42:58.815Z] 20:42:58 INFO - /builds/worker/workspace/build/src/config/rules.mk:805: recipe for target 'RDDProcessHost.i_o' failed
[task 2019-04-28T20:42:58.815Z] 20:42:58 ERROR - make[5]: *** [RDDProcessHost.i_o] Error 1
[task 2019-04-28T20:42:58.815Z] 20:42:58 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/ipc'
[task 2019-04-28T20:42:58.815Z] 20:42:58 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'dom/media/ipc/target' failed
[task 2019-04-28T20:42:58.815Z] 20:42:58 ERROR - make[4]: *** [dom/media/ipc/target] Error 2
[task 2019-04-28T20:42:58.815Z] 20:42:58 INFO - make[4]: *** Waiting for unfinished jobs....
[task 2019-04-28T20:42:58.815Z] 20:42:58 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/bindings'
[task 2019-04-28T20:42:58.815Z] 20:42:58 INFO - dom/bindings/StructuredClone.i_o

Flags: needinfo?(mfroman)

(In reply to Raul Gurzau (:RaulGurzau) from comment #7)

Hi Michael, uplifted this to beta and got a conflict. Tried to fix it, these are my changes: https://hg.mozilla.org/releases/mozilla-beta/rev/9d654bcc7cae40a6158827030d6ce179ca2392a9#l1.12

Raul, it looks like you grabbed a little too much of the surrounding code, but if I understand Comment 8, it looks like Sebastian has now landed this with the intended code. Please let me know if you need more info from me.

Flags: needinfo?(mfroman)
You need to log in before you can comment on or make changes to this bug.