1705308 - Remove intermediary buffers in MozStackWalkThread

Status: RESOLVED FIXED

(Core :: mozglue, task, P2)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Currently MozStackWalkThread uses on-stack buffers to get stack frames from the static function WalkStackMain64, then if the buffers were too small it resizes them on the stack and calls WalkStackMain64 again, and finally iterates through the buffers and invokes the user's callback for each frame.
This is quite convoluted, and I don't see the reason not to invoke the callback straight from the stack-walking code, so we wouldn't need these temporary expandable buffers.
Maybe historically there was some critical code where it would be dangerous to call arbitrary callbacks? Mike, would you know? (I will r? you anyway, you may comment then...)

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1705308 - Invoke callback from WalkStackMain64 instead of going through expandable on-stack buffers - r?glandium

Depends on D112168

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1705308 - Move WalkStackMain64 code into MozStackWalkThread - r?glandium

Depends on D112169

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1705308 - Remove WalkStackData and clean up MozStackWalkThread - r?glandium

Depends on D112170

Comment 4

[Mike Hommey [:glandium]]

So, the current complexity is a result of the history of this file. WalkStackMain64's lineage goes back to DumpStackToFileThread, which was added in bug 297723 to apparently fix problems on Windows XP.

Bug 391321 did some more because of dead locks if you got stack traces during static initialization.

The whole "send a message to a dedicated thread in order for it to stack walk" deal was removed in bug 1452204, but a lot of the complexity remained.

I'll need to do another pass, but at quick glance, this looks like a good cleanup.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2641848b9639
Invoke callback from WalkStackMain64 instead of going through expandable on-stack buffers - r=glandium
https://hg.mozilla.org/integration/autoland/rev/22e0032fcbbf
Move WalkStackMain64 code into MozStackWalkThread - r=glandium
https://hg.mozilla.org/integration/autoland/rev/94b3a2ab4ffa
Remove WalkStackData and clean up MozStackWalkThread - r=glandium

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/2641848b9639
https://hg.mozilla.org/mozilla-central/rev/22e0032fcbbf
https://hg.mozilla.org/mozilla-central/rev/94b3a2ab4ffa