Skip to content

Commit

Permalink
module: don't cache uninitialized builtins
Browse files Browse the repository at this point in the history
Don't cache the exported values of fully uninitialized builtins.
This works by adding an additional `loading` flag that is only
active during initial loading of an internal module and checking
that either the module is fully loaded or is in that state before
using its cached value.

This has the effect that builtins modules which could not be loaded
(e.g. because compilation failed due to missing stack space) can be
loaded at a later point.

Fixes: nodejs#6899
  • Loading branch information
addaleax committed May 22, 2016
1 parent 5116d98 commit 7ad588e
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 8 deletions.
23 changes: 15 additions & 8 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@
this.id = id;
this.exports = {};
this.loaded = false;
this.loading = false;
}

NativeModule._source = process.binding('natives');
Expand All @@ -368,7 +369,7 @@
}

var cached = NativeModule.getCached(id);
if (cached) {
if (cached && (cached.loaded || cached.loading)) {
return cached.exports;
}

Expand Down Expand Up @@ -432,14 +433,20 @@
var source = NativeModule.getSource(this.id);
source = NativeModule.wrap(source);

var fn = runInThisContext(source, {
filename: this.filename,
lineOffset: 0,
displayErrors: true
});
fn(this.exports, NativeModule.require, this, this.filename);
this.loading = true;

try {
var fn = runInThisContext(source, {
filename: this.filename,
lineOffset: 0,
displayErrors: true
});
fn(this.exports, NativeModule.require, this, this.filename);

this.loaded = true;
this.loaded = true;
} finally {
this.loading = false;
}
};

NativeModule.prototype.cache = function() {
Expand Down
22 changes: 22 additions & 0 deletions test/message/console_low_stack_space.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
// copy console accessor because requiring ../common touches it
const consoleDescriptor = Object.getOwnPropertyDescriptor(global, 'console');
delete global.console;
global.console = {};

require('../common');

function a() {
try {
return a();
} catch (e) {
const console = consoleDescriptor.get();
if (console.log) {
console.log('Hello, World!');
} else {
throw e;
}
}
}

a();
1 change: 1 addition & 0 deletions test/message/console_low_stack_space.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello, World!

0 comments on commit 7ad588e

Please sign in to comment.