Coredump On Close - "watcher.node"? #136264
Assignees
Labels
confirmed
Issue has been confirmed by VS Code Team member
electron
Issues and items related to Electron
file-watcher
File watcher
freeze-slow-crash-leak
VS Code crashing, performance, freeze and memory leak issues
insiders-released
Patch has been released in VS Code Insiders
linux
Issues with VS Code on Linux
Milestone
Comments
bpasero
added
freeze-slow-crash-leak
|
Can you please follow the steps in https://github.com/Microsoft/vscode/wiki/Native-Crash-Issues to get at more details around the crash and attach the result here? Thanks! |
|
Well, that's just odd. Using that command only created 2 files (client_id) and 1 folder (exthost Crash Reports). Both times I ran the commands with 2 sets of parameters, they crashed on closing. |
|
@bpasero watcher child process would be missing the crash reporter initialization unlike what extension host does which gets utilized by bootstrap-fork eventually. |
|
@deepak1556 hm, so we do use But this means every process that wants to collect crash reports that uses |
|
Yup that is correct, this only till we get to Electron 15 after which linux will start using crashpad and that code is not required anymore. So for now, we can just add this code to the watcher process launcher to debug this issue instead of a broader change ? |
bpasero
added a commit
that referenced
this issue
Nov 3, 2021
|
@vindicatorr I have a new insiders build ready that would produce a crash report (download). Can you run from the command line: |
|
Okay, so I tried looking at electron-minidump, but that just fails with ENOENT. |
|
@vindicatorr you should get a |
|
Yes, and that was a stackwalk of that dmp file. Does that not contain the pertinent information? |
|
Thanks for attempting to symbolicate the minidump but it is missing relevant symbol files, can you share the original dmp file ? |
deepak1556
added
confirmed
|
Symbolicated trace from the crashed thread: |
|
Steps to repro:
|
|
Based on the stack trace looks like INotifyBackend subscriptions are being accessed after the node process exit handlers have been invoked, @bpasero this reminds me of the CI test failure where we failed to unsubscribe from watcher events before exiting. |
|
@deepak1556 with these steps (see video) I cannot reproduce a crash dump, are you doing anything different:
|
|
Can you confirm from the process explorer if parcel watcher process is running ? Those steps looks solid. |
|
Yeah it runs off the shared process:
(this is in Parallels, Linux ARM Ubuntu 20) |
|
Latest insiders are from |
|
@deepak1556 I was specifically using the build I asked the OP to use and there is a folder being created for parcel watcher in the crash reporter dir. |
|
@vindicatorr can you share how you found that crash, which command you used? Thanks. |
|
Sorry missed that, let me try with that build. @vindicatorr can correct me but it is tracked via
|
Yup, I can get reports from the patched build. |
|
I missed the earlier message, but yeah, I didn't have to install systemd-coredump manually since it came with the systemd archlinux package. For me (again, not having to do anything), my core_pattern is Mind you, I'd be expecting that you won't get a coredump if your |
|
I can reproduce on Linux x64, but not ARM. Some facts:
|
|
@deepak1556 I am not able to reproduce this with bare node.js, so it must be Electron specific. I think you indicated this to me already back when we looked into the CI crash. Is it possible that Electron can fix this? |
|
Looks like calling Anyway, we should probably leave this issue open to at least understand the difference in runtime behaviour between Electron and node.js. |
bpasero
added a commit
that referenced
this issue
Nov 8, 2021
* watcher - stop service on Linux (#136264) * watcher - cleanup * only dispose in electron
|
@bpasero the difference wrt electron might be because without process reuse electron usually leaks node related handlers on process shutdown, hence I think the native module didn't get a chance for a proper cleanup based on node environment teardown. Once we update to Electron 15 were we will have process reuse by default, normal teardown of node environment is ensured https://github.com/electron/electron/blob/main/shell/renderer/electron_renderer_client.cc#L160 Do you want me to test with removing the above workaround as part of our Electron 15 branch ? |
|
Ah yeah that makes sense, I think we are fine then once we move to E15. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
and others
Issue Type: Bug
VS Code version: Code - Insiders 1.62.0-insider (ff1e16e, 2021-10-29T07:49:33.336Z)
OS version: Linux x64 5.13.1-dirty
Restricted Mode: No
System Info
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
rasterization: disabled_software
skia_renderer: enabled_on
video_decode: disabled_software
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Extensions (14)
A/B Experiments
It looks like it may be happening with every "close" and I didn't notice it until now.
I see 4 crashes since the 30th when I upgraded(/deleted & replaced) from 1.58.
It happened again just now after I re-opened it, only to generate this report, then closed it, with what looks to be the very same kind of dump.
The text was updated successfully, but these errors were encountered: