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.

Sign up for GitHub

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

Jump to bottom

Fix worker threads crash #1367

Merged
merged 1 commit into from
Sep 6, 2020
Merged

Fix worker threads crash #1367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ local.env
.eslintrc.js
setup.sh
/build-tmp-napi-v3
/build-tmp-napi-v6
32 changes: 16 additions & 16 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,63 +75,63 @@ matrix:
# electron Linux
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="8.2.0"
env: NODE_VERSION="12.13.0" ELECTRON_VERSION="8.2.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="8.1.0"
env: NODE_VERSION="12.13.0" ELECTRON_VERSION="8.1.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="8.0.0"
env: NODE_VERSION="12.13.0" ELECTRON_VERSION="8.0.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="7.2.0"
env: NODE_VERSION="12.8.1" ELECTRON_VERSION="7.2.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="7.1.0"
env: NODE_VERSION="12.8.1" ELECTRON_VERSION="7.1.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="7.0.0"
env: NODE_VERSION="12.8.1" ELECTRON_VERSION="7.0.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="6.1.0"
env: NODE_VERSION="12.4.0" ELECTRON_VERSION="6.1.0"
dist: trusty
addons:
apt:
sources: [ 'ubuntu-toolchain-r-test','llvm-toolchain-precise-3.5', 'gcc-multilib', 'g++-multilib', 'libsqlite3-dev:i386' ]
packages: [ 'clang-3.5', 'libstdc++-4.9-dev']
- os: linux
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="6.0.0"
env: NODE_VERSION="12.4.0" ELECTRON_VERSION="6.0.0"
dist: trusty # needed for libc6 / 'version `GLIBC_2.17` not found' error on precise
addons:
apt:
Expand All @@ -140,28 +140,28 @@ matrix:
# electron MacOs
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="8.2.0"
env: NODE_VERSION="12.13.0" ELECTRON_VERSION="8.2.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="8.1.0"
env: NODE_VERSION="12.13.0" ELECTRON_VERSION="8.1.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="8.0.0"
env: NODE_VERSION="12.13.0" ELECTRON_VERSION="8.0.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="7.2.0"
env: NODE_VERSION="12.8.1" ELECTRON_VERSION="7.2.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="7.1.0"
env: NODE_VERSION="12.8.1" ELECTRON_VERSION="7.1.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="7.0.0"
env: NODE_VERSION="12.8.1" ELECTRON_VERSION="7.0.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="6.1.0"
env: NODE_VERSION="12.4.0" ELECTRON_VERSION="6.1.0"
- os: osx
compiler: clang
env: NODE_VERSION="12" ELECTRON_VERSION="6.0.0"
env: NODE_VERSION="12.4.0" ELECTRON_VERSION="6.0.0"

env:
global:
Expand Down
34 changes: 17 additions & 17 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,88 +21,88 @@ environment:
- nodejs_version: 14
platform: x86
# electron
- nodejs_version: 12
- nodejs_version: 12.13.0
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 8.2.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.13.0
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 8.2.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.13.0
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 8.1.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.13.0
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 8.1.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.13.0
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 8.0.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.13.0
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 8.0.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.8.1
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 7.2.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.8.1
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 7.2.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.8.1
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 7.1.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.8.1
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 7.1.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.8.1
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 7.0.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.8.1
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 7.0.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.4.0
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 6.1.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.4.0
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 6.1.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.4.0
platform: x64
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 6.0.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers
- nodejs_version: 12
- nodejs_version: 12.4.0
platform: x86
NODE_RUNTIME: electron
NODE_RUNTIME_VERSION: 6.0.0
TOOLSET_ARGS: --dist-url=https://electronjs.org/headers

os: Visual Studio 2015
image: Visual Studio 2017


install:
Expand Down
5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"remote_path": "./{name}/v{version}/{toolset}/",
"package_name": "napi-v{napi_build_version}-{platform}-{arch}.tar.gz",
"napi_versions": [
3
3,
6
]
},
"contributors": [
Expand All @@ -40,7 +41,7 @@
"url": "git://github.com/mapbox/node-sqlite3.git"
},
"dependencies": {
"node-addon-api": "2.0.0",
"node-addon-api": "^3.0.0",
"node-pre-gyp": "^0.11.0"
},
"devDependencies": {
Expand Down
9 changes: 3 additions & 6 deletions scripts/build-appveyor.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,22 @@ SET EL=0

ECHO ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ %~f0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

IF /I "%msvs_toolset%"=="" ECHO msvs_toolset unset, defaulting to 14 && SET msvs_toolset=14
IF /I "%msvs_version%"=="" ECHO msvs_version unset, defaulting to 2015 && SET msvs_version=2015
IF /I "%msvs_version%"=="" ECHO msvs_version unset, defaulting to 2017 && SET msvs_version=2017

SET PATH=%CD%;%PATH%
IF "%msvs_toolset%"=="12" SET msvs_version=2013
IF NOT "%NODE_RUNTIME%"=="" SET "TOOLSET_ARGS=%TOOLSET_ARGS% --runtime=%NODE_RUNTIME%"
IF NOT "%NODE_RUNTIME_VERSION%"=="" SET "TOOLSET_ARGS=%TOOLSET_ARGS% --target=%NODE_RUNTIME_VERSION%"

ECHO APPVEYOR^: %APPVEYOR%
ECHO nodejs_version^: %nodejs_version%
ECHO platform^: %platform%
ECHO msvs_toolset^: %msvs_toolset%
ECHO msvs_version^: %msvs_version%
ECHO TOOLSET_ARGS^: %TOOLSET_ARGS%

ECHO activating VS command prompt
:: NOTE this call makes the x64 -> X64
IF /I "%platform%"=="x64" ECHO x64 && CALL "C:\Program Files (x86)\Microsoft Visual Studio %msvs_toolset%.0\VC\vcvarsall.bat" amd64
IF /I "%platform%"=="x86" ECHO x86 && CALL "C:\Program Files (x86)\Microsoft Visual Studio %msvs_toolset%.0\VC\vcvarsall.bat" x86
IF /I "%platform%"=="x64" ECHO x64 && CALL "C:\Program Files (x86)\Microsoft Visual Studio\%msvs_version%\Community\VC\Auxiliary\Build\vcvarsall.bat" amd64
IF /I "%platform%"=="x86" ECHO x86 && CALL "C:\Program Files (x86)\Microsoft Visual Studio\%msvs_version%\Community\VC\Auxiliary\Build\vcvarsall.bat" x86
IF %ERRORLEVEL% NEQ 0 GOTO ERROR

ECHO using compiler^: && CALL cl
Expand Down
6 changes: 0 additions & 6 deletions src/backup.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@

using namespace node_sqlite3;

Napi::FunctionReference Backup::constructor;


Napi::Object Backup::Init(Napi::Env env, Napi::Object exports) {
Napi::HandleScope scope(env);

Expand All @@ -24,9 +21,6 @@ Napi::Object Backup::Init(Napi::Env env, Napi::Object exports) {
InstanceAccessor("retryErrors", &Backup::RetryErrorGetter, &Backup::RetryErrorSetter),
});

constructor = Napi::Persistent(t);
constructor.SuppressDestruct();

exports.Set("Backup", t);
return exports;
}
Expand Down
2 changes: 0 additions & 2 deletions src/backup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ namespace node_sqlite3 {
*/
class Backup : public Napi::ObjectWrap<Backup> {
public:
static Napi::FunctionReference constructor;

static Napi::Object Init(Napi::Env env, Napi::Object exports);

struct Baton {
Expand Down
9 changes: 9 additions & 0 deletions src/database.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
#include <string.h>
#include <napi.h>

#include "macros.h"
#include "database.h"
#include "statement.h"

using namespace node_sqlite3;

#if NAPI_VERSION < 6
Napi::FunctionReference Database::constructor;
#endif

Napi::Object Database::Init(Napi::Env env, Napi::Object exports) {
Napi::HandleScope scope(env);
Expand All @@ -23,8 +26,14 @@ Napi::Object Database::Init(Napi::Env env, Napi::Object exports) {
InstanceAccessor("open", &Database::OpenGetter, nullptr)
});

#if NAPI_VERSION < 6
constructor = Napi::Persistent(t);
constructor.SuppressDestruct();
#else
Napi::FunctionReference* constructor = new Napi::FunctionReference();
*constructor = Napi::Persistent(t);
env.SetInstanceData<Napi::FunctionReference>(constructor);
#endif

exports.Set("Database", t);
return exports;
Expand Down
8 changes: 8 additions & 0 deletions src/database.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,23 @@ class Database;

class Database : public Napi::ObjectWrap<Database> {
public:
#if NAPI_VERSION < 6
static Napi::FunctionReference constructor;
#endif
static Napi::Object Init(Napi::Env env, Napi::Object exports);

static inline bool HasInstance(Napi::Value val) {
Napi::Env env = val.Env();
Napi::HandleScope scope(env);
if (!val.IsObject()) return false;
Napi::Object obj = val.As<Napi::Object>();
#if NAPI_VERSION < 6
return obj.InstanceOf(constructor.Value());
#else
Napi::FunctionReference* constructor =
env.GetInstanceData<Napi::FunctionReference>();
return obj.InstanceOf(constructor->Value());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check constructor isn't null before dereferencing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should never be null though. It's set in the Init.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if HasInstance is called for a val which doesn't have instance data in its env?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the val's env would be the same env that we add the instance data to. The environment is shared by all the objects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, there's no way of passing an object with a different env - that would have to be from a different worker thread which would require serialization.

#endif
}

struct Baton {
Expand Down
5 changes: 0 additions & 5 deletions src/statement.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

using namespace node_sqlite3;

Napi::FunctionReference Statement::constructor;

Napi::Object Statement::Init(Napi::Env env, Napi::Object exports) {
Napi::HandleScope scope(env);

Expand All @@ -23,9 +21,6 @@ Napi::Object Statement::Init(Napi::Env env, Napi::Object exports) {
InstanceMethod("finalize", &Statement::Finalize_),
});

constructor = Napi::Persistent(t);
constructor.SuppressDestruct();

exports.Set("Statement", t);
return exports;
}
Expand Down
2 changes: 0 additions & 2 deletions src/statement.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ typedef Row Parameters;

class Statement : public Napi::ObjectWrap<Statement> {
public:
static Napi::FunctionReference constructor;

static Napi::Object Init(Napi::Env env, Napi::Object exports);
static Napi::Value New(const Napi::CallbackInfo& info);

Expand Down