Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash reporter should not attach to terminalProcess #118767

Closed
michelleangela opened this issue Mar 11, 2021 · 9 comments
Closed

Crash reporter should not attach to terminalProcess #118767

michelleangela opened this issue Mar 11, 2021 · 9 comments
Assignees
Labels
electron Issues and items related to Electron feature-request Request for new features or functionality upstream Issue identified as 'upstream' component related (exists outside of VS Code)

Comments

@michelleangela
Copy link

michelleangela commented Mar 11, 2021

  • VS Code Version:
    Version: 1.54.1
    Commit: f30a9b7
    Date: 2021-03-04T22:45:03.974Z
    Electron: 11.3.0
    Chrome: 87.0.4280.141
    Node.js: 12.18.3
    V8: 8.7.220.31-electron.0
    OS: Darwin arm64 20.2.0

  • OS Version: macOS 11.1 Big Sur

Pre-requisite:

  • clang++ is installed on macOS ARM64 (or install xcode)

Steps to Reproduce:

  1. Create a simple cpp source file that crashes.
    Example code that crashes:
#include <iostream>

int main()
{
#if __arm64
std::cout << "arm64" << std::endl;
#endif

    std::cout << "test crash" << std::endl;
    long *p = 0; 
    *p = 0xFFFFFFFF;
    return 0;
}
  1. compile cpp source file with clang++
  2. run compiled executable from VS Code terminal shell zsh.
  3. executable should crash and crash dump should be reported under ~/Library/Logs/DiagnosticReports

Actual result:
no crash dump reported under ~/Library/Logs/DiagnosticReports

Expected result:
crash dump reported under ~/Library/Logs/DiagnosticReports

Note:
When running the sample executable from regular macOS terminal, crash dumps are reported.

Does this issue occur when all extensions are disabled?: Yes

@Colengms
Copy link
Contributor

Note that this repro's with any child process in VS Code's process tree. We see that a native process launched by our extension is not getting processed by macOS only on Mac ARM. The repro in VS Code's terminal seemed like a simpler repo of the same issue.

Possibly related is the long-standing issue that Electron was not capturing crashes of child processes. That appears to recently have been fixed? electron/electron#22552 If so, it's unclear why we are seeing child processes launched by VS Code's terminal getting captured only on Mac ARM, while still getting processed by the OS when using a Mac x64 build of the same version of VS Code.

If the Electron issue is indeed fixed, and child process crashes are being intercepted by Electron, please let us know how to enroll in whatever process will now be gathering our crashes. We have been relying on these crashes being handled by the OS so we can collect these stacks from reports logged by the OS.

@meganrogge meganrogge assigned deepak1556 and unassigned meganrogge Mar 29, 2021
@deepak1556
Copy link
Collaborator

deepak1556 commented Mar 29, 2021

Child process are now monitored by the crash handler from runtime, you can obtain the reports following the steps https://github.com/microsoft/vscode/wiki/Native-Crash-Issues#creating-and-symbolicating-local-crash-reports. Once the app is quit, the crash report will be under <crash-reporter-directory>/pending. To symbolicate, you can use https://github.com/nornagon/electron-minidump/blob/master/README.md and the symbols need to generated in the following format https://github.com/google/breakpad/blob/master/README.ANDROID#L93-L113. I will update the documentation on this in the wiki.

You can always disable the electron crash handler as an alternative, following these steps:

* Start the app with --disable-crash-reporter

or

* Open Command Palette
* Configure Runtime Arguments
* "enable-crash-reporter": false

@deepak1556 deepak1556 added the debt Code quality issues label Mar 29, 2021
@deepak1556 deepak1556 added this to the April 2021 milestone Mar 29, 2021
@Colengms
Copy link
Contributor

Hi @deepak1556 . Note that this issue is not related to child processes launched by VS Code itself or an extension. This issue occurs when the user launches unrelated processes from the Terminal. I would not expect those to be handled by the crash reporter.

@deepak1556
Copy link
Collaborator

That is a side effect of shell process being launched from vscode https://github.com/microsoft/vscode/blob/main/src/vs/platform/terminal/node/terminalProcess.ts#L198, the crash handler will attach to it. There is no way in the runtime currently to ignore a particular process, until it is fixed the above workaround should be utilized.

@deepak1556 deepak1556 added bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron terminal Integrated terminal issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) and removed debt Code quality issues labels Mar 29, 2021
@deepak1556 deepak1556 changed the title crash dumps not created when running executable from VS Code's terminal on Apple Silicon Crash reporter should not attach to terminalProcess Mar 29, 2021
@deepak1556 deepak1556 modified the milestones: April 2021, May 2021 Apr 28, 2021
@deepak1556
Copy link
Collaborator

https://github.com/chromium/crashpad/blob/master/doc/overview_design.md#the-crashpad-client explains how crashpad components work. On macOS, the handler and client rely on mach ports to communicate with each other. The client in the main process gets an exception port monitored by the handler and this exception port will get inherited by all child processes. During a crash, the OS will notify the handler because of this exception port instead of the system crash handler.

Because of the way executables are launched from the integrated terminal or from the extension host, all these child process get the same exception port. To avoid being monitored by the crashpad handler, we need to clear this exception port before launching the child process, luckily we can control the child process spawn code and we had recently switched to use posix_spawn on macOS and with posix_spawnattr_setexceptionports_np we can clear the exception port set by the crashpad client.

@deepak1556 deepak1556 removed this from the May 2021 milestone Jun 1, 2021
@deepak1556
Copy link
Collaborator

Ignoring the crashpad handler connection to process spawned from the integrated terminal depends on microsoft/node-pty#476

@Tyriar Tyriar removed the terminal Integrated terminal issues label Dec 9, 2022
@deepak1556 deepak1556 added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Dec 12, 2022
@vscodenpa vscodenpa added this to the Backlog Candidates milestone Dec 12, 2022
@vscodenpa
Copy link

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodenpa
Copy link

This feature request has not yet received the 20 community upvotes it takes to make to our backlog. 10 days to go. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodenpa
Copy link

🙁 In the last 60 days, this feature request has received less than 20 community upvotes and we closed it. Still a big Thank You to you for taking the time to create this issue! To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodenpa vscodenpa closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
electron Issues and items related to Electron feature-request Request for new features or functionality upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
Development

No branches or pull requests

6 participants