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: #6899

Ref: #6899
PR-URL: #6907
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
addaleax authored and Myles Borins committed Jun 30, 2016
1 parent 73ceb7c commit 5ba807a
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
20 changes: 13 additions & 7 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,7 @@
this.id = id;
this.exports = {};
this.loaded = false;
this.loading = true;
}

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

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

Expand Down Expand Up @@ -952,13 +953,18 @@
var source = NativeModule.getSource(this.id);
source = NativeModule.wrap(source);

var fn = runInThisContext(source, {
filename: this.filename,
lineOffset: 0
});
fn(this.exports, NativeModule.require, this, this.filename);
this.loading = true;
try {
var fn = runInThisContext(source, {
filename: this.filename,
lineOffset: 0
});
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 5ba807a

Please sign in to comment.