Closed Bug 1452204 Opened 7 years ago Closed 6 years ago

Does synchronous stackwalking in profiler_get_backtrace not work on Windows?

Categories

(Core :: mozglue, defect, P2)

Product:
Core
Component:
mozglue
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: mstange, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Bug 1452204 part 1 - Correctly set walkCallingThread.
(deleted), text/x-review-board-request
glandium
: review+
Details
Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread.
(deleted), text/x-review-board-request
glandium
: review+
Details
In this profile: https://perfht.ml/2GZCPDA there are a few styles and reflow markers which have cause callstacks. However, the C++ stacks in there look rather odd. They are all only two functions deep: profiler_get_backtrace() xul.pdb RtlAllocateMemoryBlockLookaside ntdll.pdb Is synchronous unwinding of the native stack just broken on Windows at the moment?
I think Xidron said he would try to take a look at this when he has some free cycles?
Flags: needinfo?(xidorn+moz)
Priority: -- → P2
OK, I guess I know what the problem is.
Assignee: nobody → xidorn+moz
Flags: needinfo?(xidorn+moz)
Component: Gecko Profiler → mozglue
Comment on attachment 8983010 [details] Bug 1452204 part 1 - Correctly set walkCallingThread. https://reviewboard.mozilla.org/r/248864/#review255274
Attachment #8983010 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8983011 [details] Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread. https://reviewboard.mozilla.org/r/248866/#review255282
Attachment #8983011 - Flags: review?(mh+mozilla) → review+
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/72fc40daf6cd part 1 - Correctly set walkCallingThread. r=glandium https://hg.mozilla.org/integration/autoland/rev/4431cecd4c2d part 2 - Use RtlCaptureContext to capture context for current thread. r=glandium
Backout by ebalazs@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a47957da1983 Backed out 2 changesets for perma failing in memory/replace/dmd/test/test_dmd.js
Backed out 2 changesets (bug 1452204) for perma failing in memory/replace/dmd/test/test_dmd.js Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4431cecd4c2d43fedc0acfa5016ef89a583d87b6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable&selectedJob=181841156 Backout: Backed out 2 changesets (bug 1452204) for perma failing in memory/replace/dmd/test/test_dmd.js Failure log: Backed out 2 changesets (bug 1452204) for perma failing in memory/replace/dmd/test/test_dmd.js
Flags: needinfo?(xidorn+moz)
OK, eventually I figured out what's going on. It seems we can remove quite a bit of code for walking the current thread.
Flags: needinfo?(xidorn+moz)
Comment on attachment 8983011 [details] Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread. This change is non-trivial and definitely needs another review.
Attachment #8983011 - Flags: review+ → review?(mh+mozilla)
Comment on attachment 8983011 [details] Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread. https://reviewboard.mozilla.org/r/248866/#review256098 ::: commit-message-4087b:8 (Diff revision 2) > +GetThreadContext() returns a context pointing to its own frame when it > +gets called with the current thread handle. That frame can go away after > +it returns. This patch instead uses RtlCaptureContext(), which captures > +the context of its caller, when walking the current thread. > + > +In the past, we also use a walker thread when nullptr is passed in for s/use/used/ ::: commit-message-4087b:10 (Diff revision 2) > +it returns. This patch instead uses RtlCaptureContext(), which captures > +the context of its caller, when walking the current thread. > + > +In the past, we also use a walker thread when nullptr is passed in for > +aThread, but the check doesn't cover all the cases, and having another > +thread is apparently more complicated then this approach. s/then/than/
Attachment #8983011 - Flags: review?(mh+mozilla) → review+
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6df1a71c5fe2 part 1 - Correctly set walkCallingThread. r=glandium https://hg.mozilla.org/integration/autoland/rev/12b8065ee9fc part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread. r=glandium
https://hg.mozilla.org/mozilla-central/rev/6df1a71c5fe2 https://hg.mozilla.org/mozilla-central/rev/12b8065ee9fc
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1468129
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: