Skip to content

Commit 04a4a03

Browse files
ofrobotsMyles Borins
authored and
Myles Borins
committed
test: fix flakiness of stringbytes-external
The tests used to rely on precise timing of when a JavaScript object would be garbage collected to ensure that there is enough memory available on the system. Switch the test to use a malloc/free pair instead. Ref: #5945 Ref: #6039 Ref: #6073 PR-URL: #6705 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent cfa3bea commit 04a4a03

11 files changed

+99
-49
lines changed

test/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ Tests for when the `--abort-on-uncaught-exception` flag is used.
1010

1111
### addons
1212

13-
Tests for [addon](https://nodejs.org/api/addons.html) functionality.
13+
Tests for [addon](https://nodejs.org/api/addons.html) functionality along with
14+
some tests that require an addon to function properly.
15+
1416

1517
| Runs on CI |
1618
|:----------:|
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#include <stdlib.h>
2+
#include <node.h>
3+
#include <v8.h>
4+
5+
void EnsureAllocation(const v8::FunctionCallbackInfo<v8::Value> &args) {
6+
v8::Isolate* isolate = args.GetIsolate();
7+
uintptr_t size = args[0]->IntegerValue();
8+
v8::Local<v8::Boolean> success;
9+
10+
void* buffer = malloc(size);
11+
if (buffer) {
12+
success = v8::Boolean::New(isolate, true);
13+
free(buffer);
14+
} else {
15+
success = v8::Boolean::New(isolate, false);
16+
}
17+
args.GetReturnValue().Set(success);
18+
}
19+
20+
void init(v8::Local<v8::Object> target) {
21+
NODE_SET_METHOD(target, "ensureAllocation", EnsureAllocation);
22+
}
23+
24+
NODE_MODULE(binding, init);
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
'targets': [
3+
{
4+
'target_name': 'binding',
5+
'sources': [ 'binding.cc' ]
6+
}
7+
]
8+
}
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
// v8 fails silently if string length > v8::String::kMaxLength
@@ -14,19 +14,21 @@ if (!common.enoughTestMem) {
1414
console.log(skipMessage);
1515
return;
1616
}
17-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
const maxString = buf.toString('binary');
3234
assert.equal(maxString.length, kStringMaxLength);
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('ascii');
3335
}, /toString failed/);
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('base64');
3335
}, /toString failed/);

test/sequential/test-stringbytes-external-exceed-max-by-1-binary.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('binary');
3335
}, /toString failed/);
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('hex');
3335
}, /toString failed/);

test/sequential/test-stringbytes-external-exceed-max-by-1-utf8.js renamed to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 1);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString();
3335
}, /toString failed|Invalid array buffer length/);
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,23 +10,25 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength + 2);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
const maxString = buf.toString('utf16le');
3234
assert.equal(maxString.length, (kStringMaxLength + 2) / 2);
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
2-
// Flags: --expose-gc
32

4-
const common = require('../common');
3+
const common = require('../../common');
4+
const binding = require('./build/Release/binding');
55
const assert = require('assert');
66

77
const skipMessage =
@@ -10,24 +10,26 @@ if (!common.enoughTestMem) {
1010
console.log(skipMessage);
1111
return;
1212
}
13-
assert(typeof gc === 'function', 'Run this test with --expose-gc');
1413

1514
// v8 fails silently if string length > v8::String::kMaxLength
1615
// v8::String::kMaxLength defined in v8.h
1716
const kStringMaxLength = process.binding('buffer').kStringMaxLength;
1817

1918
try {
2019
var buf = new Buffer(kStringMaxLength * 2 + 2);
21-
// Try to allocate memory first then force gc so future allocations succeed.
22-
new Buffer(2 * kStringMaxLength);
23-
gc();
2420
} catch (e) {
2521
// If the exception is not due to memory confinement then rethrow it.
2622
if (e.message !== 'Invalid array buffer length') throw (e);
2723
console.log(skipMessage);
2824
return;
2925
}
3026

27+
// Ensure we have enough memory available for future allocations to succeed.
28+
if (!binding.ensureAllocation(2 * kStringMaxLength)) {
29+
console.log(skipMessage);
30+
return;
31+
}
32+
3133
assert.throws(function() {
3234
buf.toString('utf16le');
3335
}, /toString failed/);

0 commit comments

Comments
 (0)