Clearing require.cache of native addon makes it throw when required later.

[bengl]

• Version: 5.10.1 (going back to around 0.12, I think)
• Platform: Darwin 14.5.0 Darwin Kernel Version 14.5.0: Wed Jul 29 02:26:53 PDT 2015; root:xnu-2782.40.9~1/RELEASE_X86_64 x86_64 i386
• Subsystem: module/require

require-ing a native addon after it has already been required and then removed from the require cache results in an error, rather than simply providing the module again.

For example, I inserted the following code into a test called require-cache-clear, modelled after test/addons/hello-world, using a dummy native module:

'use strict'
require('../../common')
const assert = require('assert')
const nativeMod = './build/Release/binding'

require(nativeMod)
delete require.cache[require.resolve(nativeMod)]
assert.doesNotThrow(()=>require(nativeMod))

The resulting error is:

Error: Module did not self-register.
    at Error (native)
    at Object.Module._extensions..node (module.js:443:18)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:313:12)
    at Module.require (module.js:366:17)
    at require (internal/module.js:16:19)
    at assert.doesNotThrow (/Users/bengl/node/test/addons/require-cache-clear/test.js:8:25)
    at _tryBlock (assert.js:305:5)
    at _throws (assert.js:324:12)
    at Function.assert.doesNotThrow (assert.js:352:3)

A quick Google search informs me that such errors are usually the result of version mismatches, and a node-gyp rebuild will solve them. This isn't the case here.

We came across this due to mockery's clearing of the cache. I suspect that folks using this or other modules that manipulate require.cache/Module._cache are going to (or have already) run into this at some point or another.

My best guess is that this might be related to this comment here by @bnoordhuis.

[Fishrock123]

Modifying module cache isn't exactly supported. Would be possible for the module in question to be a bit more intelligent about clearing the cache?

[bengl]

Modifying the cache is certainly documented, though perhaps incompletely at the moment. Lots of modules remove items from require.cache for testing purposes (I really can't think of other use cases).

I'm not sure what "more intelligent" would mean here. The use case is being able to require the same native addon a second time, after clearing its cache entry in-between. Seems pretty normal to me, as that's how several frameworks for mocking/stubbing work.

[bnoordhuis]

Reloading native add-ons currently isn't supported: the shared object isn't unloaded when you delete it from the cache because it would be unsafe to do so in many cases (unsafe as in segfault unsafe) and most add-ons aren't suitable for reloading anyway. They expect to be loaded once and only once; they don't expect to get unloaded.

[benjamingr]

I think we should just document the fact it's not supported. This is very hard to get right for native modules anyway.

[Fishrock123]

Modifying the cache is certainly documented

oh hmm, I looked through the document and didn't see it, sorry! D:

I'm not sure what "more intelligent" would mean here.

This is me presuming there is a way to tell something is an addon, I'm not sure beyond that.

Alternative silly idea: we make require.cache a getter/setter and return a proxy, so that people can't unload native modules for now.

Honestly though, maybe that's unnecessary safety. We should at least document it.

Maybe native addons could fall back to a second internal cache that can't be cleared?

[benjamingr]

Alternative silly idea: we make require.cache a getter/setter and return a proxy, so that people can't unload native modules for now.

Emitting a warning when trying to unload a native module could do wonders.

[bnoordhuis]

What's wrong with just documenting it?

[benjamingr]

@bnoordhuis Nothing is wrong with just documenting it, it would be an improvement already - I think that emitting a warning as well would be far more useful though since it would give users very fast feedback about what went wrong in their code.

Since Node now has warning infrastructure it makes sense to utilize it for this purpose.

[bnoordhuis]

Only if it can be done without gross hacks and performance regressions - which I'm somewhat skeptical about.

[vkurchatkin]

Decided to try cache approach, PTAL