WiseLibs / better-sqlite3

The fastest and simplest library for SQLite3 in Node.js.
MIT License
5.37k stars 394 forks source link

Crash when using prebuilt v8.5.2 with Electron 26.1.0 #1044

Open linonetwo opened 1 year ago

linonetwo commented 1 year ago

Error message before crash is

[20860:0821/201632.615257:ERROR:child_thread_impl.cc(235)] Invalid PlatformChannel receive right

Code is

      let database: Sqlite3Database.Database;
      try {
        database = new Sqlite3Database(':memory:', { verbose: logger.debug, nativeBinding: SQLITE_BINARY_PATH });
      } catch (error) {
        logger.error(`error when loading sqlite3 for workspace ${workspaceID}, skip because of error to prevent crash: ${(error as Error).message}`);
        return;
      }

It was working in 8.4.0 + "electron": "25.2.0",

Not working with 8.5.1 + "electron": "26.0.0", or 8.4.0 + + "electron": "26.0.0",

So it is electron's falut.

linonetwo commented 1 year ago

https://github.com/electron/electron/issues/39600

Using electron fiddle, I found

  const Sqlite3Database = require('better-sqlite3')
  const db = new Sqlite3Database(':memory:', { verbose: console.log, nativeBinding: '/Users/linonetwo/Desktop/repo/TiddlyGit-Desktop/node_modules/better-sqlite3/build/Release/better_sqlite3.node' });

won't crash. But closing the db will:

  //  ↓THIS Line cause problem! ↓
  db.close();

But I'm curious, why db.close() will cause the crash?


It's different, in my app, let database = new Sqlite3Database(':memory:', { nativeBinding: SQLITE_BINARY_PATH }); does the crash, but in electron fiddle, db.close() does it...

mceachen commented 1 year ago

Thanks for reporting, @linonetwo . @neoxpert contributed more context in #1053.

mceachen commented 1 year ago

Using @neoxpert 's repo, manually rebuilding fixes the issue:

npm install
cd node_modules/better-sqlite3
npx node-gyp rebuild --debug --build-from-source --runtime=electron --target=26.1.0 --dist-url=https://electronjs.org/headers
cd ../..
npm run start

Including --debug, omitting --debug, or replacing --debug with --release all result in a non-crashing better_sqlite3.node.

So it looks like the pre-built binary doesn't work correctly.

linonetwo commented 1 year ago

But wasn't the prebuilt binary generated in Github action in the same cli command? Can you rerun action to publish a new one?

So is this a bug on Electron or here, do I need to close the issue there?

mceachen commented 1 year ago

I've kicked off a rebuild for the latest version, but I suspect the same binaries will be emitted. I'll look into it a bit more tomorrow. @JoshuaWise if you've got any suggestions, that'd be great.

mceachen commented 1 year ago

Harumph--rebuilding resulted in the same behavior. Until this is properly resolved, I'd run https://github.com/WiseLibs/better-sqlite3/issues/1044#issuecomment-1693735596 in your postinstall.

SrIzan10 commented 1 year ago

I got this different error: [28380:0828/232451.103:ERROR:crashpad_client_win.cc(844)] not connected and it looks like the workaround solved it.

NathanaelA commented 1 year ago

Just a small warning, one issue I discovered was that the machine you compile Better-SQLITE on's GLIBC version matters GREATLY. If you use the above command line on say a Ubuntu 22.04 machine to rebuild it, your GLIB is 2.35 which means your app will crash on a fairly large percentage of all Linux machines, since a large number of them are running a version of GLIBC that is before 2.35.

I actually have a special build docker container I use to build better-sqlite (everytime I upgrade my electron) using GLIBC 2.28 because I ran into way too many people still having a GLIB 2.28 - 2.30 machine and the prebuilt version is GLIBC 2.31. You don't need to go any farther back than 2.28, because Node 16 (iirc) requires GLIB 2.28 and higher to run, meaning most versions of Electron requires GLIBC 2.28 and higher to run.

neoxpert commented 1 year ago

Looking through the build logs I just found those lines for the Windows and Ubuntu builds:

Windows line 855 + 856 prebuild info build Preparing to prebuild better-sqlite3@8.6.0 for electron 26.0.0 on win32-x64 using node-gyp prebuild info build prebuilds\better-sqlite3-v8.6.0-electron-v116-win32-x64.tar.gz exists, skipping build

Ubuntu line 942 + 943 prebuild info build Preparing to prebuild better-sqlite3@8.6.0 for electron 26.0.0 on linux-x64 using node-gyp prebuild info build prebuilds/better-sqlite3-v8.6.0-electron-v116-linux-x64.tar.gz exists, skipping build

May it be that Electron 25 and 26 share the same NODE_MODULE_VERSION number, or at least the same number is resolved in the build process? Electron 25 and 26 both load the exact same prebuilt binary, thus leading to the crash on Electron 26 being somehow "expected" ...

Prinzhorn commented 1 year ago

May it be that Electron 25 and 26 share the same NODE_MODULE_VERSION number, or at least the same number is resolved in the build process? Electron 25 and 26 both load the exact same prebuilt binary, thus leading to the crash on Electron 26 being somehow "expected" ...

They do https://github.com/electron/node-abi/blob/f3efb50137fada3613298cf0b6055e9c4fe8ba6a/abi_registry.json#L255-L268

We had this exact same thing happen with Electron 14. Back then I was making the hypothesis that relying on the ABI version alone is not enough as a criteria to identify prebuilds, see https://github.com/WiseLibs/better-sqlite3/issues/694#issuecomment-912506229 If this is the case, then the whole prebuild system is broken for Electron and this is an issue greater than better-sqlite3 itself. It might just be that native modules are rarely used and maybe other native modules don't trigger the crash because of the APIs they use.

I'm not sure if back then we actually solved this or if we just sat it out until Electron 16 was released.

Prinzhorn commented 1 year ago

I opened https://github.com/prebuild/prebuild/issues/309 to have this discussion in the right place.

NoxFly commented 1 year ago

Same issue here, Using any package (Enmap, better-sqlite3-multiple-ciphers, ...) that uses better-sqlite3 will run into a SIGSEGV when only creating the database. This occurs only when using Electron.

I'm on Ubuntu 23.04, Node v18.17, Electron 26. But this seems to occur with any OS, Node >= 14, Electron >= 11 (of what I've found on other repo issues).

coltoneshaw commented 12 months ago

I did the steps https://github.com/WiseLibs/better-sqlite3/issues/1044#issuecomment-1693735596 and it throws this error on the build process. I assume ignore it?

Fixed by adding the below to the package.json file and overriding rebuild's dep

  "resolutions": {
    "node-abi": "^3.47.0"
  }

To ensure your native dependencies are always matched electron version, simply add script `"postinstall": "electron-builder install-app-deps" to your `package.json`
  • writing effective config  file=dist/builder-effective-config.yaml
  • rebuilding native dependencies  dependencies=better-sqlite3@8.6.0 platform=darwin arch=x64
  • install prebuilt binary  name=better-sqlite3 version=8.6.0 platform=darwin arch=x64 napi=
  • build native dependency from sources  name=better-sqlite3
                                          version=8.6.0
                                          platform=darwin
                                          arch=x64
                                          napi=
                                          reason=prebuild-install failed with error (run with env DEBUG=electron-builder to get more information)
                                          error=/Users/coltonshaw/Desktop/scripts/pcomvp-desktop/node_modules/prebuild-install/node_modules/node-abi/index.js:39
      throw new Error('Could not detect abi for version ' + target + ' and runtime ' + runtime + '.  Updating "node-abi" might help solve this issue if it is a new release of ' + runtime)
      ^

    Error: Could not detect abi for version 26.1.0 and runtime electron.  Updating "node-abi" might help solve this issue if it is a new release of electron
        at getAbi (/Users/coltonshaw/Desktop/scripts/pcomvp-desktop/node_modules/prebuild-install/node_modules/node-abi/index.js:39:9)
        at module.exports (/Users/coltonshaw/Desktop/scripts/pcomvp-desktop/node_modules/prebuild-install/rc.js:54:57)
        at Object.<anonymous> (/Users/coltonshaw/Desktop/scripts/pcomvp-desktop/node_modules/prebuild-install/bin.js:8:27)
        at Module._compile (node:internal/modules/cjs/loader:1233:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1287:10)
        at Module.load (node:internal/modules/cjs/loader:1091:32)
        at Module._load (node:internal/modules/cjs/loader:938:12)
        at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
        at node:internal/main/run_main_module:23:47

    Node.js v20.5.1

  • rebuilding native dependency  name=better-sqlite3 version=8.6.0
  • packaging       platform=darwin arch=x64 electron=26.1.0 appOutDir=dist/mac
neoxpert commented 12 months ago

No, you can't ignore that. Something in your build environment / project seems to be incorrect. Looks like the node-abi module resolved is not up to date.

coltoneshaw commented 12 months ago

No, you can't ignore that. Something in your build environment / project seems to be incorrect. Looks like the node-abi module resolved is not up to date.

That's what I thought too, but it's just the packaged version of rebuild that has node abi 3.3.0.

Funny thing, the final build works perfectly though.

coltoneshaw commented 12 months ago

If it's relevant for anyone, the above commands I made an arch adjustment and it seems to work. I have to build mainly for x64 and was yelling at me about arm.

npx node-gyp rebuild --debug --build-from-source --runtime=electron --target=26.1.0 --dist-url=https://electronjs.org/headers --arch=x64  
Prinzhorn commented 11 months ago

Looks like migrating to N-API (#271) could resolve these issue in the future, as pointed out by the prebuild maintainer

numan commented 11 months ago

If it's relevant for anyone, the above commands I made an arch adjustment and it seems to work. I have to build mainly for x64 and was yelling at me about arm.

npx node-gyp rebuild --debug --build-from-source --runtime=electron --target=26.1.0 --dist-url=https://electronjs.org/headers --arch=x64  

Thanks, this worked for me. However, I'm still having issues with running it inside a Utility Process. The process just crashes right away with no log output that I can see. The main process which launches the utility process works fine.

numan commented 11 months ago

If it's relevant for anyone, the above commands I made an arch adjustment and it seems to work. I have to build mainly for x64 and was yelling at me about arm.

npx node-gyp rebuild --debug --build-from-source --runtime=electron --target=26.1.0 --dist-url=https://electronjs.org/headers --arch=x64  

Thanks, this worked for me. However, I'm still having issues with running it inside a Utility Process. The process just crashes right away with no log output that I can see. The main process which launches the utility process works fine.

Electron 27.0.0 seems to have resolved this issue for me.