Skip to content

Commit 9080ba9

Browse files
zertoshzertosh
committed
Fix input rows race condition
1 parent 52af8f8 commit 9080ba9

File tree

10 files changed

+286
-11
lines changed

10 files changed

+286
-11
lines changed

‎index.js

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ function Deps (opts) {
5858
// modules in it. If not, set it anyway so it's defined later.
5959
if (!this.options.expose) this.options.expose = {};
6060
this.pending = 0;
61+
this.inputPending = 0;
6162

6263
var topfile = path.join(this.basedir, '__fake.js');
6364
this.top = {
@@ -298,10 +299,13 @@ Deps.prototype.walk = function (id, parent, cb) {
298299
this.pending ++;
299300

300301
var rec = {};
302+
var input;
301303
if (typeof id === 'object') {
302304
rec = copy(id);
303305
if (rec.entry === false) delete rec.entry;
304306
id = rec.file || rec.id;
307+
input = true;
308+
this.inputPending ++;
305309
}
306310

307311
self.resolve(id, parent, function (err, file, pkg) {
@@ -319,6 +323,7 @@ Deps.prototype.walk = function (id, parent, cb) {
319323

320324
if (opts.postFilter && !opts.postFilter(id, file, pkg)) {
321325
if (--self.pending === 0) self.push(null);
326+
if (input) --self.inputPending;
322327
return cb(null, undefined);
323328
}
324329
if (err && rec.source) {
@@ -333,12 +338,14 @@ Deps.prototype.walk = function (id, parent, cb) {
333338
}
334339
if (err && self.options.ignoreMissing) {
335340
if (--self.pending === 0) self.push(null);
341+
if (input) --self.inputPending;
336342
self.emit('missing', id, parent);
337343
return cb && cb(null, undefined);
338344
}
339345
if (err) return self.emit('error', err);
340346
if (self.visited[file]) {
341347
if (-- self.pending === 0) self.push(null);
348+
if (input) --self.inputPending;
342349
return cb && cb(null, file);
343350
}
344351
self.visited[file] = true;
@@ -380,18 +387,23 @@ Deps.prototype.walk = function (id, parent, cb) {
380387
};
381388
var resolved = {};
382389

383-
deps.forEach(function (id) {
384-
if (opts.filter && !opts.filter(id)) {
385-
resolved[id] = false;
386-
if (--p === 0) done();
387-
return;
388-
}
389-
self.walk(id, current, function (err, r) {
390-
resolved[id] = r;
391-
if (--p === 0) done();
390+
if (input) --self.inputPending;
391+
392+
(function resolve () {
393+
if (self.inputPending > 0) return setTimeout(resolve);
394+
deps.forEach(function (id) {
395+
if (opts.filter && !opts.filter(id)) {
396+
resolved[id] = false;
397+
if (--p === 0) done();
398+
return;
399+
}
400+
self.walk(id, current, function (err, r) {
401+
resolved[id] = r;
402+
if (--p === 0) done();
403+
});
392404
});
393-
});
394-
if (deps.length === 0) done();
405+
if (deps.length === 0) done();
406+
})();
395407

396408
function done () {
397409
if (!rec.id) rec.id = file;

‎test/cache.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
var parser = require('../');
2+
var test = require('tape');
3+
var path = require('path');
4+
5+
var files = {
6+
foo: path.join(__dirname, '/files/foo.js'),
7+
bar: path.join(__dirname, '/files/bar.js')
8+
};
9+
10+
var sources = {
11+
foo: 'notreal foo',
12+
bar: 'notreal bar'
13+
};
14+
15+
var cache = {};
16+
cache[files.foo] = {
17+
source: sources.foo,
18+
deps: { './bar': files.bar }
19+
};
20+
cache[files.bar] = {
21+
source: sources.bar,
22+
deps: {}
23+
};
24+
25+
test('uses cache', function (t) {
26+
t.plan(1);
27+
var p = parser({ cache: cache });
28+
p.end({ id: 'foo', file: files.foo, entry: false });
29+
30+
var rows = [];
31+
p.on('data', function (row) { rows.push(row) });
32+
p.on('end', function () {
33+
t.same(rows.sort(cmp), [
34+
{
35+
id: 'foo',
36+
file: files.foo,
37+
source: sources.foo,
38+
deps: { './bar': files.bar }
39+
},
40+
{
41+
id: files.bar,
42+
file: files.bar,
43+
source: sources.bar,
44+
deps: {}
45+
}
46+
].sort(cmp));
47+
});
48+
});
49+
50+
function cmp (a, b) { return a.id < b.id ? -1 : 1 }

‎test/cache_expose.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
var parser = require('../');
2+
var test = require('tape');
3+
var path = require('path');
4+
5+
var files = {
6+
foo: path.join(__dirname, '/files/foo.js'),
7+
bar: path.join(__dirname, '/files/bar.js')
8+
};
9+
10+
var sources = {
11+
foo: 'notreal foo',
12+
bar: 'notreal bar'
13+
};
14+
15+
var cache = {};
16+
cache[files.foo] = {
17+
source: sources.foo,
18+
deps: { './bar': files.bar }
19+
};
20+
cache[files.bar] = {
21+
source: sources.bar,
22+
deps: {}
23+
};
24+
25+
test('cache preserves expose and entry', function (t) {
26+
t.plan(1);
27+
var p = parser({ cache: cache });
28+
p.write({ id: files.bar, expose: 'bar2', entry: false });
29+
p.end({ id: 'foo', file: files.foo, entry: true, expose: 'foo2' });
30+
31+
var rows = [];
32+
p.on('data', function (row) { rows.push(row) });
33+
p.on('end', function () {
34+
t.same(rows.sort(cmp), [
35+
{
36+
id: 'foo',
37+
expose: 'foo2',
38+
entry: true,
39+
file: files.foo,
40+
source: sources.foo,
41+
deps: { './bar': files.bar }
42+
},
43+
{
44+
id: files.bar,
45+
expose: 'bar2',
46+
file: files.bar,
47+
source: sources.bar,
48+
deps: {}
49+
}
50+
].sort(cmp));
51+
});
52+
});
53+
54+
function cmp (a, b) { return a.id < b.id ? -1 : 1 }

‎test/cache_partial.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
var parser = require('../');
2+
var test = require('tape');
3+
var fs = require('fs');
4+
var path = require('path');
5+
6+
var files = {
7+
foo: path.join(__dirname, '/files/foo.js'),
8+
bar: path.join(__dirname, '/files/bar.js')
9+
};
10+
11+
var sources = {
12+
foo: 'notreal foo',
13+
bar: fs.readFileSync(files.bar, 'utf8')
14+
};
15+
16+
var cache = {};
17+
cache[files.foo] = {
18+
source: sources.foo,
19+
deps: { './bar': files.bar }
20+
};
21+
22+
test('uses cache and reads from disk', function (t) {
23+
t.plan(1);
24+
var p = parser({ cache: cache });
25+
p.end({ id: 'foo', file: files.foo, entry: false });
26+
27+
var rows = [];
28+
p.on('data', function (row) { rows.push(row) });
29+
p.on('end', function () {
30+
t.same(rows.sort(cmp), [
31+
{
32+
id: 'foo',
33+
file: files.foo,
34+
source: sources.foo,
35+
deps: { './bar': files.bar }
36+
},
37+
{
38+
id: files.bar,
39+
file: files.bar,
40+
source: sources.bar,
41+
deps: {}
42+
}
43+
].sort(cmp));
44+
});
45+
});
46+
47+
function cmp (a, b) { return a.id < b.id ? -1 : 1 }

‎test/cache_partial_expose.js

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
var parser = require('../');
2+
var test = require('tape');
3+
var fs = require('fs');
4+
var path = require('path');
5+
var copy = require('shallow-copy');
6+
7+
var files = {
8+
abc: path.join(__dirname, '/expose/lib/abc.js'),
9+
xyz: path.join(__dirname, '/expose/lib/xyz.js'),
10+
foo: path.join(__dirname, '/expose/foo.js'),
11+
bar: path.join(__dirname, '/expose/bar.js'),
12+
main: path.join(__dirname, '/expose/main.js')
13+
};
14+
15+
var sources = Object.keys(files).reduce(function (acc, file) {
16+
acc[file] = fs.readFileSync(files[file], 'utf8');
17+
return acc;
18+
}, {});
19+
20+
var cache = {};
21+
cache[files.abc] = {
22+
source: sources.abc,
23+
deps: {}
24+
};
25+
cache[files.xyz] = {
26+
source: sources.xyz,
27+
deps: {'../foo': files.foo}
28+
};
29+
cache[files.foo] = {
30+
source: sources.foo,
31+
deps: {'./lib/abc': files.abc}
32+
};
33+
cache[files.bar] = {
34+
source: sources.bar,
35+
deps: {xyz: files.xyz}
36+
};
37+
cache[files.main] = {
38+
source: sources.main,
39+
deps: {
40+
abc: files.abc,
41+
xyz: files.xyz,
42+
'./bar': files.bar
43+
}
44+
};
45+
46+
test('preserves expose and entry with partial cache', function(t) {
47+
t.plan(1);
48+
49+
var partialCache = copy(cache);
50+
delete partialCache[files.bar];
51+
52+
var p = parser({ cache: partialCache });
53+
p.write({ id: 'abc', file: files.abc, expose: 'abc' });
54+
p.write({ id: 'xyz', file: files.xyz, expose: 'xyz' });
55+
p.end({ id: 'main', file: files.main, entry: true });
56+
57+
var rows = [];
58+
p.on('data', function (row) { rows.push(row); });
59+
p.on('end', function () {
60+
t.same(rows.sort(cmp), [
61+
{
62+
id: files.bar,
63+
file: files.bar,
64+
source: sources.bar,
65+
deps: {xyz: files.xyz}
66+
},
67+
{
68+
file: files.foo,
69+
id: files.foo,
70+
source: sources.foo,
71+
deps: {'./lib/abc': files.abc}
72+
},
73+
{
74+
id: 'abc',
75+
file: files.abc,
76+
source: sources.abc,
77+
deps: {},
78+
entry: true,
79+
expose: 'abc'
80+
},
81+
{
82+
id: 'main',
83+
file: files.main,
84+
source: sources.main,
85+
deps: {
86+
'./bar': files.bar,
87+
abc: files.abc,
88+
xyz: files.xyz
89+
},
90+
entry: true
91+
},
92+
{
93+
id: 'xyz',
94+
file: files.xyz,
95+
source: sources.xyz,
96+
deps: {'../foo': files.foo},
97+
entry: true,
98+
expose: 'xyz'
99+
}
100+
].sort(cmp));
101+
});
102+
});
103+
104+
function cmp (a, b) { return a.id < b.id ? -1 : 1 }

‎test/expose/bar.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
require('xyz');

‎test/expose/foo.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
require('./lib/abc');

‎test/expose/lib/abc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('abc');

‎test/expose/lib/xyz.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
require('../foo');
2+
console.log('xyz');

‎test/expose/main.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
require('abc');
2+
require('xyz');
3+
require('./bar');

0 commit comments

Comments
 (0)