Skip to content

Commit

Permalink
Support for io.js and improved stack guard
Browse files Browse the repository at this point in the history
In some pathological cases you could corrupt the stack using fibers and
cause a segfault. There seem to be some changes to how the stack guard
works from v8 3.14.x (node 0.10.x) to 3.28.x (node 0.12.x). This was
causing a lot of confusion so I brushed the problem under the rug a
little bit.
  • Loading branch information
laverdet committed Mar 26, 2015
1 parent aff726a commit 198c2f4
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 25 deletions.
32 changes: 20 additions & 12 deletions src/fibers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,17 @@ namespace uni {
isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
}

void SetResourceConstraints(Isolate* isolate, ResourceConstraints* constraints) {
v8::SetResourceConstraints(isolate, constraints);
}
#if NODE_MODULE_VERSION >= 0x002A
void SetStackGuard(Isolate* isolate, void* guard) {
isolate->SetStackLimit(reinterpret_cast<uintptr_t>(guard));
}
#else
void SetStackGuard(Isolate* isolate, void* guard) {
ResourceConstraints constraints;
constraints.set_stack_limit(reinterpret_cast<uint32_t*>(guard));
v8::SetResourceConstraints(isolate, &constraints);
}
#endif

#else
// Node v0.10.x and lower
Expand Down Expand Up @@ -247,10 +255,14 @@ namespace uni {
V8::AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
}

void SetResourceConstraints(Isolate* isolate, ResourceConstraints* constraints) {
v8::SetResourceConstraints(constraints);
void SetStackGuard(Isolate* isolate, void* guard) {
ResourceConstraints constraints;
// Extra padding for old versions of v8. Shit's fucked.
constraints.set_stack_limit(
reinterpret_cast<uint32_t*>(guard) + 18 * 1024
);
v8::SetResourceConstraints(&constraints);
}

#endif
}

Expand Down Expand Up @@ -577,13 +589,9 @@ class Fiber {
Isolate::Scope isolate_scope(that.isolate);
uni::HandleScope scope(that.isolate);

// Set the stack guard for this "thread"; allow 128k or 256k of padding past the JS limit for
// Set the stack guard for this "thread"; allow 4k of padding past the JS limit for
// native v8 code to run
ResourceConstraints constraints;
constraints.set_stack_limit(reinterpret_cast<uint32_t*>(
(size_t*)that.this_fiber->bottom() + 32 * 1024
));
uni::SetResourceConstraints(that.isolate, &constraints);
uni::SetStackGuard(that.isolate, reinterpret_cast<char*>(that.this_fiber->bottom()) + 1024 * 4);

TryCatch try_catch;
that.ClearWeak();
Expand Down
42 changes: 29 additions & 13 deletions test/stack-overflow2.js
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
var Fiber = require('fibers');
Fiber(function() {
// Because of how v8 handles strings, the call to new RegExp chews up a lot of stack space
// outside of JS.
function fn() {
var foo = '';
for (var ii = 0; ii < 1024; ++ii) {
foo += 'a';
}
new RegExp(foo, 'g');
}

// Calculate how far we can go recurse without hitting the JS stack limit
// Calculate how far we can go recurse without hitting the JS stack limit
function calculateStackSpace() {
var max = 0;
function testRecursion(ii) {
++max;
Expand All @@ -19,11 +10,36 @@ Fiber(function() {
try {
testRecursion();
} catch (err) {}
return max;
}

// Invoke a RepExp operation that eats a lot of stack space
function pathologicRegExp(preStack) {
function fn() {
var foo = '';
for (var ii = 0; ii < 1024; ++ii) {
foo += 'a';
}
new RegExp(foo, 'g');
}

// Recurse to the limit and then invoke a stack-heavy C++ operation
function wasteStack(ii) {
ii ? wasteStack(ii - 1) : fn();
}
wasteStack(max - 128); // not sure if this test is even that useful because of this padding
console.log('pass');
wasteStack(preStack);
}

Fiber(function() {

// Ensure that this doesn't ruin everything while in a fiber
var max = calculateStackSpace();
for (var stack = max; stack > 0; --stack) {
try {
pathologicRegExp(stack);
break;
} catch (err) {}
}
}).run();

console.log('pass');
36 changes: 36 additions & 0 deletions test/stack-overflow3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
var Fiber = require('fibers');

// Create a bunch of fibers and consume each fiber's stack, then yield
var fibers = [];
for (var ii = 0; ii < 100; ++ii) {
var fiber;
fibers.push(fiber = Fiber(function() {
var caught = false;
var recur = 0, stop;
function foo() {
++recur;
try {
foo();
} catch (err) {
if (!caught) {
stop = recur - 500;
caught = true;
} else if (stop === recur) {
process.stdout.write(''); // do a thing?
return Fiber.yield();
}
--recur;
throw err;
}
}
foo();
}));
fiber.run();
}

// Unwind started fibers
fibers.forEach(function(fiber) {
fiber.run();
});

console.log('pass');

0 comments on commit 198c2f4

Please sign in to comment.