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

Electron 10 - sporadic illegal access errors resulting in exit code 7 during process.exit #25267

Closed
3 tasks done
flotwig opened this issue Sep 1, 2020 · 7 comments · Fixed by #25430
Closed
3 tasks done

Comments

@flotwig
Copy link
Contributor

flotwig commented Sep 1, 2020

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

  • Electron Version:
    • 10.0.0 and above
  • Operating System:
    • Pop_OS 20.04, Debian 10
  • Last Known Working Electron version:
    • 9.2.1

Expected Behavior

process.exit(code) exits the process with exit code code.

Actual Behavior

After updating to Electron 10.0.0, process.exit(0) sporadically results in an error that looks like this:

undefined:0


illegal access
(Use `node --trace-uncaught ...` to show where the exception was thrown)

The process then exits with exit code 7.

Passing --trace-uncaught to electron seems to have no effect on the output.

To Reproduce

You can see this error popping up in the status checks on our Electron 10 upgrade PR: cypress-io/cypress#8406

I attempted to make a reproducible example for this, but couldn't reproduce it outside of Cypress's tests.

To reproduce using Cypress's tests:

  1. Clone Cypress, check out the Electron 10 branch, install dependencies:
    git clone https://github.com/cypress-io/cypress.git
    cd cypress
    git checkout renovate/electron-10.x
    yarn
    
  2. Open packages/server/test/e2e/1_deprecated_spec.ts and add a .only to the e2e.it on line 22, so it reads:
    e2e.it.only('push and no return - warns user exactly once', {
    
  3. Run the test until it fails:
    cd packages/server
    # run this until it fails:
    yarn test-e2e 1_dep --browser electron
    # if you're using zsh, this will do the trick:
    while yarn test-e2e 1_dep --browser electron; do echo 'passed'; done
    

You should see the test fail about 1/3 of the time with exit code 7 and the error mentioned above.

You can enable Cypress's debug logging by setting env var DEBUG=cypress:*, from which you can see that the last thing that happens before the crash is a call to process.exit(0).


This is blocking us from upgrading to Electron 10, so please let me know if there is a way I can investigate further, or if I can provide any additional information to help debug.

@nornagon
Copy link
Member

nornagon commented Sep 1, 2020

We've been seeing this in our CI occasionally and haven't been able to track it down.

My best current guess is that it's related to I/O (possibly to stdout?) occurring during the shutdown process.

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Sep 28, 2020

@nornagon in #25430 you didn't seem super confident that you've found the root cause. I've been able to strip down my code base to a minimal example that reproduces it 100% of the time for me. I'm using a Worker Thread but I'm not sure if it is relevent to the issue. It's just what my stripped down version came down to. I hope it helps.

Electron 10.1.2 on Ubuntu 20.04. You can also clone it from here https://github.com/Prinzhorn/electron-illegal-access

main.js

const path = require('path');
const { Worker } = require('worker_threads');
const { app, BrowserWindow } = require('electron');

let worker = new Worker(path.join(__dirname, 'server.js'));
let mainWindow = null;

app.on('ready', () => {
  mainWindow = new BrowserWindow();
});

server.js

setInterval(() => {
  console.log('hello from server');
}, 1000);

Run it using electron main.js. Once you close the window you get the error.

$ npm start

> electron-illegal-access@1.0.0 start /home/alex/Projects/issues/electron-illegal-access
> electron main.js

hello from server
hello from server
hello from server

undefined:0


illegal access
(Use `node --trace-uncaught ...` to show where the exception was thrown)
/home/alex/Projects/issues/electron-illegal-access/node_modules/electron/dist/electron main.js[44520]: ../../third_party/electron_node/src/node_platform.cc:467:std::shared_ptr<PerIsolatePlatformData> node::NodePlatform::ForIsolate(v8::Isolate *): Assertion `data' failed.
/home/alex/Projects/issues/electron-illegal-access/node_modules/electron/dist/electron exited with signal SIGABRT

@nornagon
Copy link
Member

This should be fixed by #25430, which is not yet in a 10.x release but should be shortly.

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Sep 29, 2020

I just tested with 10.1.3 and my above code still errors, but now with exited with signal SIGSEGV. Now the question is if this is a completely unrelated issue or if we just need to dig deeper? It'd be a coincidence if the same minimal example surfaces two unrelated issues, right?

I've updated it so it doesn't require user interaction (closing the window) anymore.

Also here https://github.com/Prinzhorn/electron-illegal-access

main.js

const path = require('path');
const { Worker } = require('worker_threads');
const { app, BrowserWindow } = require('electron');

let worker = new Worker(path.join(__dirname, 'server.js'));
let mainWindow = null;

app.on('ready', () => {
  mainWindow = new BrowserWindow();
  mainWindow.close();
});

server.js (can actually just be empty as well)

setInterval(() => {
  console.log('hello from server');
}, 1000);

Edit: with ELECTRON_ENABLE_LOGGING I get even more interesting intel.

[17181:0100/000000.189351:ERROR:sandbox_linux.cc(374)] InitializeSandbox() called with multiple threads in process gpu-process

Also the --type=gpu-process and --type=utility are still running after the SIGSEGV exit. They will eventually exit and I get a log on the already disconnected terminal

[18365:0100/000000.014185:INFO:child_thread_impl.cc(853)] ChildThreadImpl::EnsureConnected()

Edit2: Maybe related? cypress-io/cypress#7048 @flotwig does Cypress use worker threads as well? Can you reproduce the issue with my code above?

@nornagon
Copy link
Member

The code you've linked doesn't produce a segfault for me on master, but it does on 10.1.3. Here's the stack trace:

* thread #1, name = 'CrBrowserMain', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18)
  * frame #0: 0x000000010656bb93 Electron Framework`node::NodePlatform::AddIsolateFinishedCallback(v8::Isolate*, void (*)(void*), void*) [inlined] std::__1::shared_ptr<node::PerIsolatePlatformData>::operator bool(this=<unavailable>) const at memory:3851:68 [opt]
    frame #1: 0x000000010656bb93 Electron Framework`node::NodePlatform::AddIsolateFinishedCallback(this=0x0000000109049c20, isolate=0x0000147000000000, cb=(Electron Framework`node::Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(std::__1::shared_ptr<v8::ArrayBuffer::Allocator>)::$_10::__invoke(void*) [inlined] node::Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(std::__1::shared_ptr<v8::ArrayBuffer::Allocator>)::$_10::operator()(void*) const at env.cc:1101
Electron Framework`node::Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(std::__1::shared_ptr<v8::ArrayBuffer::Allocator>)::$_10::__invoke(void*) at env.cc:1101), data=0x0000000112d3acf0)(void*), void*) at node_platform.cc:355 [opt]
    frame #2: 0x00000001064b99c6 Electron Framework`node::Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(this=0x0000000108880000, allocator=nullptr) at env.cc:1101:15 [opt]
    frame #3: 0x00000001065a209a Electron Framework`node::worker::(anonymous namespace)::SABLifetimePartner::CleanupHook(data=<unavailable>) at sharedarraybuffer_metadata.cc:75:18 [opt]
    frame #4: 0x00000001064b58e6 Electron Framework`node::Environment::RunCleanup(this=0x0000000108880000) at env.cc:628:7 [opt]
    frame #5: 0x00000001064810be Electron Framework`node::FreeEnvironment(env=0x0000000108880000) at environment.cc:307:8 [opt]
    frame #6: 0x0000000100208336 Electron Framework`electron::ElectronBrowserMainParts::PostMainMessageLoopRun() [inlined] std::__1::default_delete<electron::NodeEnvironment>::operator(this=<unavailable>, __ptr=0x00000001090571e0)(electron::NodeEnvironment*) const at memory:2378:5 [opt]
    frame #7: 0x000000010020832e Electron Framework`electron::ElectronBrowserMainParts::PostMainMessageLoopRun() [inlined] std::__1::unique_ptr<electron::NodeEnvironment, std::__1::default_delete<electron::NodeEnvironment> >::reset(this=0x0000000109049608, __p=0x0000000000000000) at memory:2633 [opt]
    frame #8: 0x000000010020831d Electron Framework`electron::ElectronBrowserMainParts::PostMainMessageLoopRun(this=0x00000001090495c0) at electron_browser_main_parts.cc:555 [opt]
    frame #9: 0x0000000101e5e6fe Electron Framework`content::BrowserMainLoop::ShutdownThreadsAndCleanUp(this=<unavailable>) at browser_main_loop.cc:1040:13 [opt]
    frame #10: 0x0000000101e60081 Electron Framework`content::BrowserMainRunnerImpl::Shutdown(this=0x0000000109046b00) at browser_main_runner_impl.cc:178:17 [opt]
    frame #11: 0x0000000101e5b8cd Electron Framework`content::BrowserMain(parameters=0x00007ffeefbfede8) at browser_main.cc:49:16 [opt]

@nornagon
Copy link
Member

I've opened a new issue to track this: #25691

@flotwig
Copy link
Contributor Author

flotwig commented Oct 1, 2020

Edit2: Maybe related? cypress-io/cypress#7048 @flotwig does Cypress use worker threads as well? Can you reproduce the issue with my code above?

@Prinzhorn no, at least not in the officially-supported features those folks were using. We do have some experimental functionality that relies on worker_threads, which was runnin into a similar segfault issue with Electron 9: #23315

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants