Skip to content

Commit dd5510c

Browse files
rmnilinmscdex
rmnilin
authored and
mscdex
committed
lib: fix workers thread-safety issues
Fixes: #1393
1 parent 0fe2643 commit dd5510c

File tree

2 files changed

+73
-24
lines changed

2 files changed

+73
-24
lines changed

‎lib/protocol/crypto/src/binding.cc‎

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,15 @@ class ChaChaPolyCipher : public ObjectWrap {
8484
SetPrototypeMethod(tpl, "encrypt", Encrypt);
8585
SetPrototypeMethod(tpl, "free", Free);
8686

87-
constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
87+
Local<Function> func = Nan::GetFunction(tpl).ToLocalChecked();
88+
Local<Context> context = Nan::GetCurrentContext();
89+
v8::Isolate* isolate = context->GetIsolate();
90+
91+
constructor().Set(isolate, func);
8892

8993
Nan::Set(target,
9094
Nan::New("ChaChaPolyCipher").ToLocalChecked(),
91-
Nan::GetFunction(tpl).ToLocalChecked());
95+
func);
9296
}
9397

9498
private:
@@ -387,8 +391,8 @@ class ChaChaPolyCipher : public ObjectWrap {
387391
obj->clear();
388392
}
389393

390-
static inline Nan::Persistent<Function> & constructor() {
391-
static Nan::Persistent<Function> my_constructor;
394+
static inline v8::Eternal<v8::Function> & constructor() {
395+
static v8::Eternal<v8::Function> my_constructor;
392396
return my_constructor;
393397
}
394398

@@ -414,11 +418,15 @@ class AESGCMCipher : public ObjectWrap {
414418
SetPrototypeMethod(tpl, "encrypt", Encrypt);
415419
SetPrototypeMethod(tpl, "free", Free);
416420

417-
constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
421+
Local<Function> func = Nan::GetFunction(tpl).ToLocalChecked();
422+
Local<Context> context = Nan::GetCurrentContext();
423+
v8::Isolate* isolate = context->GetIsolate();
424+
425+
constructor().Set(isolate, func);
418426

419427
Nan::Set(target,
420428
Nan::New("AESGCMCipher").ToLocalChecked(),
421-
Nan::GetFunction(tpl).ToLocalChecked());
429+
func);
422430
}
423431

424432
private:
@@ -633,8 +641,8 @@ class AESGCMCipher : public ObjectWrap {
633641
obj->clear();
634642
}
635643

636-
static inline Nan::Persistent<Function> & constructor() {
637-
static Nan::Persistent<Function> my_constructor;
644+
static inline v8::Eternal<v8::Function> & constructor() {
645+
static v8::Eternal<v8::Function> my_constructor;
638646
return my_constructor;
639647
}
640648

@@ -651,11 +659,15 @@ class GenericCipher : public ObjectWrap {
651659
SetPrototypeMethod(tpl, "encrypt", Encrypt);
652660
SetPrototypeMethod(tpl, "free", Free);
653661

654-
constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
662+
Local<Function> func = Nan::GetFunction(tpl).ToLocalChecked();
663+
Local<Context> context = Nan::GetCurrentContext();
664+
v8::Isolate* isolate = context->GetIsolate();
665+
666+
constructor().Set(isolate, func);
655667

656668
Nan::Set(target,
657669
Nan::New("GenericCipher").ToLocalChecked(),
658-
Nan::GetFunction(tpl).ToLocalChecked());
670+
func);
659671
}
660672

661673
private:
@@ -1014,8 +1026,8 @@ class GenericCipher : public ObjectWrap {
10141026
obj->clear();
10151027
}
10161028

1017-
static inline Nan::Persistent<Function> & constructor() {
1018-
static Nan::Persistent<Function> my_constructor;
1029+
static inline v8::Eternal<v8::Function> & constructor() {
1030+
static v8::Eternal<v8::Function> my_constructor;
10191031
return my_constructor;
10201032
}
10211033

@@ -1044,11 +1056,15 @@ class ChaChaPolyDecipher : public ObjectWrap {
10441056
SetPrototypeMethod(tpl, "decryptLen", DecryptLen);
10451057
SetPrototypeMethod(tpl, "free", Free);
10461058

1047-
constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
1059+
Local<Function> func = Nan::GetFunction(tpl).ToLocalChecked();
1060+
Local<Context> context = Nan::GetCurrentContext();
1061+
v8::Isolate* isolate = context->GetIsolate();
1062+
1063+
constructor().Set(isolate, func);
10481064

10491065
Nan::Set(target,
10501066
Nan::New("ChaChaPolyDecipher").ToLocalChecked(),
1051-
Nan::GetFunction(tpl).ToLocalChecked());
1067+
func);
10521068
}
10531069

10541070
private:
@@ -1440,8 +1456,8 @@ class ChaChaPolyDecipher : public ObjectWrap {
14401456
obj->clear();
14411457
}
14421458

1443-
static inline Nan::Persistent<Function> & constructor() {
1444-
static Nan::Persistent<Function> my_constructor;
1459+
static inline v8::Eternal<v8::Function> & constructor() {
1460+
static v8::Eternal<v8::Function> my_constructor;
14451461
return my_constructor;
14461462
}
14471463

@@ -1468,11 +1484,15 @@ class AESGCMDecipher : public ObjectWrap {
14681484
SetPrototypeMethod(tpl, "decrypt", Decrypt);
14691485
SetPrototypeMethod(tpl, "free", Free);
14701486

1471-
constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
1487+
Local<Function> func = Nan::GetFunction(tpl).ToLocalChecked();
1488+
Local<Context> context = Nan::GetCurrentContext();
1489+
v8::Isolate* isolate = context->GetIsolate();
1490+
1491+
constructor().Set(isolate, func);
14721492

14731493
Nan::Set(target,
14741494
Nan::New("AESGCMDecipher").ToLocalChecked(),
1475-
Nan::GetFunction(tpl).ToLocalChecked());
1495+
func);
14761496
}
14771497

14781498
private:
@@ -1697,8 +1717,8 @@ class AESGCMDecipher : public ObjectWrap {
16971717
obj->clear();
16981718
}
16991719

1700-
static inline Nan::Persistent<Function> & constructor() {
1701-
static Nan::Persistent<Function> my_constructor;
1720+
static inline v8::Eternal<v8::Function> & constructor() {
1721+
static v8::Eternal<v8::Function> my_constructor;
17021722
return my_constructor;
17031723
}
17041724

@@ -1716,11 +1736,15 @@ class GenericDecipher : public ObjectWrap {
17161736
SetPrototypeMethod(tpl, "decrypt", Decrypt);
17171737
SetPrototypeMethod(tpl, "free", Free);
17181738

1719-
constructor().Reset(Nan::GetFunction(tpl).ToLocalChecked());
1739+
Local<Function> func = Nan::GetFunction(tpl).ToLocalChecked();
1740+
Local<Context> context = Nan::GetCurrentContext();
1741+
v8::Isolate* isolate = context->GetIsolate();
1742+
1743+
constructor().Set(isolate, func);
17201744

17211745
Nan::Set(target,
17221746
Nan::New("GenericDecipher").ToLocalChecked(),
1723-
Nan::GetFunction(tpl).ToLocalChecked());
1747+
func);
17241748
}
17251749

17261750
private:
@@ -2183,8 +2207,8 @@ class GenericDecipher : public ObjectWrap {
21832207
obj->clear();
21842208
}
21852209

2186-
static inline Nan::Persistent<Function> & constructor() {
2187-
static Nan::Persistent<Function> my_constructor;
2210+
static inline v8::Eternal<v8::Function> & constructor() {
2211+
static v8::Eternal<v8::Function> my_constructor;
21882212
return my_constructor;
21892213
}
21902214

‎test/test-worker-imports.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Test for thread-safety issues caused by subsequent imports of the module
2+
// in worker threads: https://github.com/mscdex/ssh2/issues/1393.
3+
// Each subsequent worker increases probability of abnormal termination.
4+
// The probability of a false pass due to zero response becomes negligible
5+
// for 4 consecutive workers.
6+
'use strict';
7+
8+
let Worker, isMainThread;
9+
try {
10+
({ Worker, isMainThread } = require('worker_threads'));
11+
} catch (e) {
12+
if (e.code !== 'MODULE_NOT_FOUND') throw e;
13+
process.exit(0);
14+
}
15+
require('../lib/index.js');
16+
17+
if (isMainThread) {
18+
async function runWorker() {
19+
return new Promise((r) => new Worker(__filename).on('exit', r));
20+
}
21+
runWorker()
22+
.then(runWorker)
23+
.then(runWorker)
24+
.then(runWorker);
25+
}

0 commit comments

Comments
 (0)