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

SIGBUS error using mksnapshot on v22.0.0 and above #91

Open
ryanthemanuel opened this issue Mar 26, 2023 · 9 comments
Open

SIGBUS error using mksnapshot on v22.0.0 and above #91

ryanthemanuel opened this issue Mar 26, 2023 · 9 comments

Comments

@ryanthemanuel
Copy link

I have a script that I've been able to create a snapshot with versions of electron-mksnapshot under v22.0.0. However, when I upgrade to v22.0.0, mksnapshot crashes. I don't really get any kinds of logs with the crash, but I can print out details on the mksnapshot process and it shows it is crashing with a SIGBUS error, e.g.:

Error running mksnapshot. {
  status: null,
  signal: 'SIGBUS',
  output: [ null, null, null ],
  pid: 74231,
  stdout: null,
  stderr: null
}

My script is fairly large, and I am wondering if there is some kind of new memory limit I am running up against. If I comment out certain parts of an object that is being created in the script mksnapshot will succeed; however it seems to be more tied to the number of properties of the object rather than anything specific about the properties themselves. Is there something new in v22.0.0 that I might be running into? Is there any setting I can try to fiddle around with?

I am still digging into this and will keep updating as I discover things, but thought I'd check here to see if anyone has any ideas.

@ckerr
Copy link
Member

ckerr commented Mar 28, 2023

Hi @ryanthemanuel!

Planned breaking changes to Electron are listed out in https://www.electronjs.org/docs/latest/breaking-changes. I don't see anything there about memory changes in 22, though if you're upgrading from <21 it's possible that it's related to 21's memory cage changes. Maybe that helps you to triage?

If fixing EOM in your script isn't the issue, please update this ticket with more information and ideally a small testcase that can be used to reproduce the problem. I get it that this ticket is mostly about asking for ideas than reporting a bug; but if you get further into this and do think it's a bug, the maintainers will need more actionable info in order to do triage on this.

Thanks for understanding, and good luck! I hope this helps.

@ryanthemanuel
Copy link
Author

Hi @ckerr. Thanks for the response.

I ended up trying the latest version of electron-mksnapshot (v23.2.1) just to see if that fixes the problem and it fails differently but still fails. I'm running:

node_modules/electron-mksnapshot/mksnapshot.js snapshot.js

I'm still working through a minimal example, but if it helps here's the full file that's causing the problem:

snapshot - does not work.js.zip

If I end up commenting enough lines mksnapshot works again:

snapshot - works.js.zip

From that file, I ended up uncommenting one line and it crashes again:

snapshot - works - one require difference.js.zip

Sorry I don't have a smaller example available, but I'm still working through the problem and thought I'd share info as I come across it.

@mjhenkes
Copy link

mjhenkes commented May 2, 2023

I'm taking a look at this issue. Same as @ryanthemanuel above I've been able to recreate the issue and it's a bit strange given our large snapshot file I can 'fix' the problem by commenting out some of the exports, however building the snapshot with just the commented out export also succeeds, so there appears to be either some relationship between the exports or possibly the issue has to due with the number of exports?

I can confirm that the snapshot builds without failures in electron 21.4.4 and begins throwing errors in 22.0.0

Is there a better way to get the proper error logged out of the mksnapshot command? I've tried all these flags but none have shed light on the actual error that prevents mksnapshot from completing:

  • --log-all
  • --detailed-error-stack-trace
  • --log-source-code
  • --log-source-position
  • --log-code
  • --log-feedback-vector
  • --log-code-disassemble
  • --log-function-events
  • --trace
  • --stack-trace-on-illegal
  • --print-all-exceptions
  • --heap-profiler-trace-objects
  • --trace-compilation-dependencies

My next steps are to try and build electron-mksnapshot 22.0.0 with the version of chrome included in electron-mksnapshot 21.4.4, assuming I can make that work I'll binary search my way through versions to see which version of chrome may have introduced this issue.

@mjhenkes
Copy link

mjhenkes commented May 4, 2023

I've been able to narrow down the introduction of this bug to this commit: electron/electron@08ccc81#diff-4cc955bcf0b472353786cc8d8b7d346fb432c091bed099de884885e938392814

That commit updates the chromium version and patches. The previous commit succeeds.

this implies that the issue was 'introduced by chromium between 106.0.5249.199 and 107.0.5274.0' or introduced by the electron patch for the same

(I did this by building electron mksnapshot for these commits.)

Next steps are to try and further narrow down the chrome version.

@mjhenkes
Copy link

I've been able to identify the introduction of this issue as this change in V8: https://chromium.googlesource.com/v8/v8.git/+/91637c25fcdb199526a7060ceca858aeef16bd3d

v8/v8@91637c2

This was released in v8 10.7.71 and pulled into chromium 107.0.5272

V8_REMOVE_BUILTINS_CODE_OBJECTS has since been removed in this commit:
v8/v8@91869ce

Honestly I was hoping for an easier bug, We can't even patch to re-enable the global in later versions because it was removed. :(

Any Suggestions on how to proceed would be appreciated.

@mjhenkes
Copy link

I've recreated this in v8 by itself and have opened a bug https://bugs.chromium.org/p/v8/issues/detail?id=14007

@mjhenkes
Copy link

v8 has fixed this bug in v8 Version 11.6.21

@mjhenkes
Copy link

I went ahead and created a PR to patch this fix into electron: electron/electron#38490 with the hopes of getting this fixed sooner and possibly in older versions as well.

@corneliusroemer
Copy link

I think this issue can be closed then :)

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

No branches or pull requests

4 participants