Crash annotation fd is incorrect in GeckoView

RESOLVED FIXED in Firefox 60

Status

()

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jld, Assigned: Alex_Gaynor)

Tracking

unspecified
mozilla60
Unspecified
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

I noticed this in GeckoChildProcessHost::LaunchAndroidService:

  int32_t crashFd = (it != fds_to_remap.end()) ? it->first : -1;
  int32_t crashAnnotationFd = (it != fds_to_remap.end()) ? it->first : -1;

There seems to be a missing increment there, so it will use the same file descriptor for both of those.

(Also, we probably rethink how fds_to_remap works in general — hard-coding assumptions about what order the elements are in like this seems less than ideal — but that's a separate bug.)
Assignee

Updated

a year ago
Assignee: nobody → agaynor

Comment 2

a year ago
mozreview-review
Comment on attachment 8952824 [details]
Bug 1440034 - fixed reporting of annotations at the time of crashes on GeckoView;

https://reviewboard.mozilla.org/r/222054/#review227966
Attachment #8952824 - Flags: review?(rbarker) → review+
Assignee

Updated

a year ago
Keywords: checkin-needed
The patch makes things better, but it's still not 100% right, AFAICT. Judging from GeckoChildProcessHost::PerformAsyncLaunchInternal(), it's possible for the crashFd to not be inserted (e.g. if CrashReporter::IsDummy() is true), but crashAnnotationFd is unconditionally inserted.

It should probably be changed so that the two crash fds are always either both missing or both present?

Comment 4

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3237c9532102
fixed reporting of annotations at the time of crashes on GeckoView; r=rbarker
Keywords: checkin-needed
Backed out for breaking android builds.

backout: https://hg.mozilla.org/integration/autoland/rev/9ff8aa7fdf52ec4d59c82bc8c670a679ece12169

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3237c9532102e87bd57505e5aa7b48589837280d

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=163611000&repo=autoland&lineNumber=10261

[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/ipc/glue/Unified_cpp_ipc_glue0.cpp:92:
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  /builds/worker/workspace/build/src/ipc/glue/GeckoChildProcessHost.cpp:1196:23: error: expected ';' at end of declaration
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -    int32_t crashFd = -1
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -                        ^
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -                        ;
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  1 error generated.
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1047: recipe for target 'Unified_cpp_ipc_glue0.o' failed
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  make[4]: *** [Unified_cpp_ipc_glue0.o] Error 1
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/ipc/glue'
[task 2018-02-22T03:30:40.949Z] 03:30:40     INFO -  make[4]: *** Waiting for unfinished jobs....
[task 2018-02-22T03:30:40.950Z] 03:30:40     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl'
[task 2018-02-22T03:30:40.950Z] 03:30:40     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl'
Flags: needinfo?(agaynor)
See Also: → 1440207
Comment hidden (mozreview-request)
Assignee

Comment 7

a year ago
Ok, let's try this again. Try run to verify it's clean: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd081b2039a0b71bb537cffbd66e46bcf34abc44&group_state=expanded
Flags: needinfo?(agaynor)
Keywords: checkin-needed

Comment 8

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2d731c03025f
fixed reporting of annotations at the time of crashes on GeckoView; r=rbarker
Keywords: checkin-needed

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2d731c03025f
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.