From 04a4a03b396bcd1788ecd15ccf84e36bd06ca17c Mon Sep 17 00:00:00 2001
From: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Mon, 4 Apr 2016 09:09:52 -0700
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/5945
Ref: https://github.com/nodejs/node/pull/6039
Ref: https://github.com/nodejs/node/pull/6073

PR-URL: https://github.com/nodejs/node/pull/6705
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 test/README.md                                |  4 +++-
 .../binding.cc                                | 24 +++++++++++++++++++
 .../binding.gyp                               |  8 +++++++
 .../test-stringbytes-external-at-max.js       | 14 ++++++-----
 ...ingbytes-external-exceed-max-by-1-ascii.js | 14 ++++++-----
 ...ngbytes-external-exceed-max-by-1-base64.js | 14 ++++++-----
 ...ngbytes-external-exceed-max-by-1-binary.js | 14 ++++++-----
 ...tringbytes-external-exceed-max-by-1-hex.js | 14 ++++++-----
 ...ringbytes-external-exceed-max-by-1-utf8.js | 14 ++++++-----
 ...st-stringbytes-external-exceed-max-by-2.js | 14 ++++++-----
 .../test-stringbytes-external-exceed-max.js   | 14 ++++++-----
 11 files changed, 99 insertions(+), 49 deletions(-)
 create mode 100644 test/addons/stringbytes-external-exceed-max/binding.cc
 create mode 100644 test/addons/stringbytes-external-exceed-max/binding.gyp
 rename test/{parallel => addons/stringbytes-external-exceed-max}/test-stringbytes-external-at-max.js (73%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max-by-1-ascii.js (72%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max-by-1-base64.js (72%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max-by-1-binary.js (79%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max-by-1-hex.js (72%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max-by-1-utf8.js (75%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max-by-2.js (73%)
 rename test/{sequential => addons/stringbytes-external-exceed-max}/test-stringbytes-external-exceed-max.js (72%)

diff --git a/test/README.md b/test/README.md
index a40e800fef5921..73512413c02834 100644
--- a/test/README.md
+++ b/test/README.md
@@ -10,7 +10,9 @@ Tests for when the `--abort-on-uncaught-exception` flag is used.
 
 ### addons
 
-Tests for [addon](https://nodejs.org/api/addons.html) functionality.
+Tests for [addon](https://nodejs.org/api/addons.html) functionality along with
+some tests that require an addon to function properly.
+
 
 | Runs on CI |
 |:----------:|
diff --git a/test/addons/stringbytes-external-exceed-max/binding.cc b/test/addons/stringbytes-external-exceed-max/binding.cc
new file mode 100644
index 00000000000000..65390852c2d2dd
--- /dev/null
+++ b/test/addons/stringbytes-external-exceed-max/binding.cc
@@ -0,0 +1,24 @@
+#include <stdlib.h>
+#include <node.h>
+#include <v8.h>
+
+void EnsureAllocation(const v8::FunctionCallbackInfo<v8::Value> &args) {
+  v8::Isolate* isolate = args.GetIsolate();
+  uintptr_t size = args[0]->IntegerValue();
+  v8::Local<v8::Boolean> success;
+
+  void* buffer = malloc(size);
+  if (buffer) {
+    success = v8::Boolean::New(isolate, true);
+    free(buffer);
+  } else {
+    success = v8::Boolean::New(isolate, false);
+  }
+  args.GetReturnValue().Set(success);
+}
+
+void init(v8::Local<v8::Object> target) {
+  NODE_SET_METHOD(target, "ensureAllocation", EnsureAllocation);
+}
+
+NODE_MODULE(binding, init);
diff --git a/test/addons/stringbytes-external-exceed-max/binding.gyp b/test/addons/stringbytes-external-exceed-max/binding.gyp
new file mode 100644
index 00000000000000..3bfb84493f3e87
--- /dev/null
+++ b/test/addons/stringbytes-external-exceed-max/binding.gyp
@@ -0,0 +1,8 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}
diff --git a/test/parallel/test-stringbytes-external-at-max.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-at-max.js
similarity index 73%
rename from test/parallel/test-stringbytes-external-at-max.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-at-max.js
index 5467d05a20104a..a5d937d521d887 100644
--- a/test/parallel/test-stringbytes-external-at-max.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-at-max.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 // v8 fails silently if string length > v8::String::kMaxLength
@@ -14,13 +14,9 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 try {
   var buf = new Buffer(kStringMaxLength);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,5 +24,11 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 const maxString = buf.toString('binary');
 assert.equal(maxString.length, kStringMaxLength);
diff --git a/test/sequential/test-stringbytes-external-exceed-max-by-1-ascii.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js
similarity index 72%
rename from test/sequential/test-stringbytes-external-exceed-max-by-1-ascii.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js
index af47a8e779a7a4..dee30d9fef0f97 100644
--- a/test/sequential/test-stringbytes-external-exceed-max-by-1-ascii.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-ascii.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,6 +24,12 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('ascii');
 }, /toString failed/);
diff --git a/test/sequential/test-stringbytes-external-exceed-max-by-1-base64.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js
similarity index 72%
rename from test/sequential/test-stringbytes-external-exceed-max-by-1-base64.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js
index fb599875a0459b..f06052d31611fc 100644
--- a/test/sequential/test-stringbytes-external-exceed-max-by-1-base64.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-base64.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,6 +24,12 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('base64');
 }, /toString failed/);
diff --git a/test/sequential/test-stringbytes-external-exceed-max-by-1-binary.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js
similarity index 79%
rename from test/sequential/test-stringbytes-external-exceed-max-by-1-binary.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js
index f4904b672cfea7..9af82da07eb706 100644
--- a/test/sequential/test-stringbytes-external-exceed-max-by-1-binary.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,6 +24,12 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('binary');
 }, /toString failed/);
diff --git a/test/sequential/test-stringbytes-external-exceed-max-by-1-hex.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js
similarity index 72%
rename from test/sequential/test-stringbytes-external-exceed-max-by-1-hex.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js
index 9dd9d23565acf2..a1d54460a22d8e 100644
--- a/test/sequential/test-stringbytes-external-exceed-max-by-1-hex.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-hex.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,6 +24,12 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('hex');
 }, /toString failed/);
diff --git a/test/sequential/test-stringbytes-external-exceed-max-by-1-utf8.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js
similarity index 75%
rename from test/sequential/test-stringbytes-external-exceed-max-by-1-utf8.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js
index 3ff6f4978878ee..546e2a1a134fbd 100644
--- a/test/sequential/test-stringbytes-external-exceed-max-by-1-utf8.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-utf8.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength + 1);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,6 +24,12 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString();
 }, /toString failed|Invalid array buffer length/);
diff --git a/test/sequential/test-stringbytes-external-exceed-max-by-2.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-2.js
similarity index 73%
rename from test/sequential/test-stringbytes-external-exceed-max-by-2.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-2.js
index 1ac5a5c7fe1ced..94f6602cda40c6 100644
--- a/test/sequential/test-stringbytes-external-exceed-max-by-2.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-2.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength + 2);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,5 +24,11 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 const maxString = buf.toString('utf16le');
 assert.equal(maxString.length, (kStringMaxLength + 2) / 2);
diff --git a/test/sequential/test-stringbytes-external-exceed-max.js b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js
similarity index 72%
rename from test/sequential/test-stringbytes-external-exceed-max.js
rename to test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js
index 61666b8ebc6b48..41a14118cf538a 100644
--- a/test/sequential/test-stringbytes-external-exceed-max.js
+++ b/test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max.js
@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
-const common = require('../common');
+const common = require('../../common');
+const binding = require('./build/Release/binding');
 const assert = require('assert');
 
 const skipMessage =
@@ -10,7 +10,6 @@ if (!common.enoughTestMem) {
   console.log(skipMessage);
   return;
 }
-assert(typeof gc === 'function', 'Run this test with --expose-gc');
 
 // v8 fails silently if string length > v8::String::kMaxLength
 // v8::String::kMaxLength defined in v8.h
@@ -18,9 +17,6 @@ const kStringMaxLength = process.binding('buffer').kStringMaxLength;
 
 try {
   var buf = new Buffer(kStringMaxLength * 2 + 2);
-  // Try to allocate memory first then force gc so future allocations succeed.
-  new Buffer(2 * kStringMaxLength);
-  gc();
 } catch (e) {
   // If the exception is not due to memory confinement then rethrow it.
   if (e.message !== 'Invalid array buffer length') throw (e);
@@ -28,6 +24,12 @@ try {
   return;
 }
 
+// Ensure we have enough memory available for future allocations to succeed.
+if (!binding.ensureAllocation(2 * kStringMaxLength)) {
+  console.log(skipMessage);
+  return;
+}
+
 assert.throws(function() {
   buf.toString('utf16le');
 }, /toString failed/);