Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bengl opened this issue Apr 12, 2016 · 17 comments
Closed
Labels
doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem.

Comments

@bengl
Copy link
Member

bengl commented Apr 12, 2016

  • 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.

@mscdex mscdex added the module Issues and PRs related to the module subsystem. label Apr 12, 2016
@Fishrock123
Copy link
Contributor

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
Copy link
Member Author

bengl commented Apr 12, 2016

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
Copy link
Member

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
Copy link
Member

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

@Fishrock123
Copy link
Contributor

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?

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Apr 12, 2016
@benjamingr
Copy link
Member

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
Copy link
Member

What's wrong with just documenting it?

@benjamingr
Copy link
Member

@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
Copy link
Member

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

@vkurchatkin
Copy link
Contributor

Decided to try cache approach, PTAL

@vkurchatkin
Copy link
Contributor

Modifying the cache is certainly documented

To be fair, mockery still uses undocumented API (Module._cache)

@bengl
Copy link
Member Author

bengl commented Apr 12, 2016

To be fair, mockery still uses undocumented API (Module._cache)

Yes, but it could modify require.cache and have the same effect.

bengl added a commit to bengl/node that referenced this issue Apr 12, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: nodejs#6160
jasnell pushed a commit that referenced this issue Apr 18, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 19, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 21, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
joelostrowski pushed a commit to joelostrowski/node that referenced this issue Apr 25, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: nodejs#6160
PR-URL: nodejs#6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this issue Apr 26, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue May 3, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue May 6, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2016
Clarify in docs for require.cache that reloading native modules
isn't supported.

Related: #6160
PR-URL: #6168
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Dzenly
Copy link
Contributor

Dzenly commented Sep 2, 2016

Oh, how it is inconvenient for me. :(

It is like dlclose() removing from POSIX, "just for safety reasons". :(

I have created a native module which handles licenses for my soft and which can change its internal state irreversible when something is going wrong.
Now I am creating many tests for this module. I have created an JS engine which do 'require' my JS tests (which do 'require' my native addon and perform different actions). The engine collects some statistics. So I need many addon reloadings, because of addon state changing.

Now it seems I need to introduce some additional function 'clearState()' to my addon and export it in test configuration. Too inconvenient, too unflexible. :(

@bnoordhuis
Copy link
Member

It's been documented in commit 5c14d69 so I'll go ahead and close the issue.

@esr360
Copy link

esr360 commented May 27, 2019

Apologies for reviving a dead issue, I just wanted to see if there was a better way to do what I'm attempting, because I ran into this issue after I had deleted a cached file from require.cache and was presented with the following error:

Module did not self-register.

The reason I am manually removing a file from the cache is because I am building a custom Node-Sass importer to allow JavaScript files to be imported into Sass. If a file is imported into the file that is imported into Sass, and a change is made to that file, Sass does not re-compile with the update. If a file is imported directly into Sass and a change is made to that file, then it will recompile with the update. This is because require caches files, so to get around this I am deleting the files that are imported into any file that are imported into Sass from the cache, but am then presented with the error.

Is there anyway around this?

@davisalanmatt
Copy link

@esr360 The javascript files importing into Sass are all non-native correct? So you should be able to safely delete them without getting the error. Only native ones (such as the importer itself) will fail on re-requirement. Are you clearing the entire cache?

A solution I've been using to circumvent the error is to save the required module to a global as such:

global.myModule = global.myModule || require( 'myModule' );

This way it will not try and re-require if it has already been set. Using globals is never ideal but easiest solution I've found for now.

@dharijanto
Copy link

Apologies for reviving a dead issue, I just wanted to see if there was a better way to do what I'm attempting, because I ran into this issue after I had deleted a cached file from require.cache and was presented with the following error:

Module did not self-register.

The reason I am manually removing a file from the cache is because I am building a custom Node-Sass importer to allow JavaScript files to be imported into Sass. If a file is imported into the file that is imported into Sass, and a change is made to that file, Sass does not re-compile with the update. If a file is imported directly into Sass and a change is made to that file, then it will recompile with the update. This is because require caches files, so to get around this I am deleting the files that are imported into any file that are imported into Sass from the cache, but am then presented with the error.

Is there anyway around this?

This is how I WAR-ed my problem. When deleting just skip over file with .node extension.
if (cachedPath.startsWith(hostFilePath) && !cachedPath.endsWith('.node')) { let skip = false delete require.cache[cachedPath] }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

10 participants