Repository navigation
npm throws an error on master #1898
Description
Looks like a regression introduced in 6d95f4f.
$ npm
module.js:334
throw err;
^
Error: Cannot find module 'internal/smalloc'
at Function.Module._resolveFilename (module.js:332:15)
at Function.Module._load (module.js:282:25)
at Module.require (module.js:361:17)
at require (module.js:380:17)
at evalmachine.<anonymous>:22:20
at Object.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/graceful-fs/fs.js:11:1)
at Module._compile (module.js:426:26)
at Object.Module._extensions..js (module.js:444:10)
at Module.load (module.js:351:32)
at Function.Module._load (module.js:306:12)
/cc @vkurchatkin
Activity
targos commentedon Jun 5, 2015
It is caused by this change in fs.js.
It breaks graceful-fs which is evaluating the code from fs in a context where internal modules are not allowed.
targos commentedon Jun 5, 2015
/cc @isaacs
vkurchatkin commentedon Jun 5, 2015
Well, now we know the answer
fs: use `kMaxLength` from binding
thefourtheye commentedon Jun 5, 2015
I wonder why this was not caught when I ran
make testwith this change. This is not covered in testcases?bnoordhuis commentedon Jun 5, 2015
@thefourtheye
make testdoesn't runmake test-npm(yet? there was talk about that recently.)thefourtheye commentedon Jun 5, 2015
@bnoordhuis But, requiring
fsin other modules should have failed, right?bnoordhuis commentedon Jun 5, 2015
When you say 'other modules', do you mean 'core modules'? Those have access to internal modules so no problem there.
require('fs')in user code isn't affected either.The npm breakage is because graceful-fs does
eval(process.binding('natives').fs). It's completely unsupported and normally I'd say 'tough luck'. But a broken npm is bad so we'll have to add a workaround until graceful-fs gets fixed.fs: use `kMaxLength` from binding
fs: use `kMaxLength` from binding
fs: use `kMaxLength` from binding
vkurchatkin commentedon Jun 5, 2015
Fixed in 2dcef83, though this should be fixed in
graceful-fsat some point. Until then we can't use internal modules infs2 remaining items
isaacs commentedon Jun 5, 2015
@silverwind There's been talk about having graceful-fs in core. Generally, the vote has come down to a "no" (and I've voted "no" as well).
Some of the features in graceful-fs are totally fine, imo, and do belong in core. There's no reason why
fs.lchmodorfs.lchownshould be missing. If a platform doesn't treat symlinks and their targets as having separate distinct permissions and owners (ie, linux) then it should still let you go ahead and callfs.lchmodand have it just be identical tofs.chmodon the symlink. For systems that do support it, it should work as expected.The biggest feature in graceful-fs is that it detects and avoids
EMFILEerrors that occur when you open too many files at once. When using thefsmodule directly, thefs.opencall will simply fail, and it's up to you to handle the error. Withgraceful-fs, open calls that fail withEMFILEare placed in a queue, and re-attempted when some other fd is closed.This is important to programs like npm that are in the business of unpacking a lot of tarballs and reading a lot of files, and want to parallelize to the maximum amount allowed by the operating system.
However, there are situations where this behavior would be surprising, or even trigger problematic deadlocks! Rather than have an immediate failure from an
fs.opencall, you now have an indefinitely long lag.Putting that behavior in core is a bad idea. However, this highlights the point that we do need a better way for this to work.
Monkey-patching the core module caused a lot of problems because builtin modules are globals, so the change 8 levels deep in the tree could have strange impacts on consumers. (isaacs/node-graceful-fs#28)
I don't have a great solution in mind.
Fishrock123 commentedon Jun 5, 2015
Maybe we could have a
--queue-at-max-fdsort of option? It seems like we may be able to get the best of both worlds there. I can imagine a lot of programs that would rather not have errors for that, and other ones that would.I'm not sure how npm would set something like that though...
isaacs commentedon Jun 5, 2015
@Fishrock123 Really, what you want is a
fs.openthat queues until a close ifEMFILEis raised, and anotherfs.openthat merely keeps track of how many open FDs there are. The reason why graceful-fs had to switch from a monkey-patch ofrequire('fs')to a clone of it was precisely because you don't want this happening at the global level.But, then you need to make sure that
fs.readdirhandles EMFILE and tracks fd counts as well, and that all the calls to open fromfs.readFileandfs.writeFileand the stream classes all do the right thing.One approach would be for it to lock the code down at a particular version of
process.binding('natives').fs, but then if the internals change, there's a higher likelihood of other more subtle errors cropping up.Maybe we ought to split fs out into a separate module entirely, and vendor it into node core?
silverwind commentedon Jun 6, 2015
Wow, this discussion is a bit over my head, but my prime reason for using
graceful-fsis the EACCESS/EPERM handling on Windows, related to locked files by other programs which I recall were randomly happening even without using any AV software present and which were breaking my programs in all sort of unexpected ways.It surprises me a bit that we don't get any bug reports in core regarding Windows file locking, maybe the issue isn't as widespread as I thought it is. Anyways, I'd probably be happy if we can get this feature alone into core, if it has no drawbacks (which it probably has).
bnoordhuis commentedon Jun 6, 2015
Just the first thing that pops into my mind: an application that silently stalls because of a deferred file operation will be much, much harder to debug than one that simply throws an error.
On a philosophical level, graceful-fs-like behavior in core is way too much policy in the "mechanism, not policy" sense.
Please God, no.
isaacs commentedon Jun 6, 2015
Well, you need to be sure that when this application opens files, it will close them promptly when it's done with them.
Though, the same could be said of getting EMFILE or ENFILE errors on sockets, and we handle those gracefully. Why do it there and not for files?
bnoordhuis commentedon Jun 6, 2015
I'm not sure what you mean. The fd-stashed-away-for-hard-times trick? That's really just a hack to avoid busy looping with level-triggered listen sockets. Comparing that with graceful-fs is giving it way too much credit. :-)
fs: add a temporary fix for re-evaluation support
fs: add a temporary fix for re-evaluation support