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

Plugins modules are not correctly unloaded on uninstall (or updates) #3940

Closed
JohnXLivingston opened this issue Apr 8, 2021 · 6 comments
Closed
Assignees
Labels
Component: PeerTube Plugin 📦 Features that can be developed in a plugin, but require PeerTube plugin API development Type: Bug 🐛 Confirmed bug, at least replicated once by another contributor

Comments

@JohnXLivingston
Copy link
Contributor

Describe the current behavior

While working on new plugins, I have sometimes weird behaviors that requires a peertube restart.
I thinks that it is because of these lines:

const modulePath = join(pluginPath, packageJSON.library)
delete require.cache[modulePath]
const library: PluginLibrary = require(modulePath)

Plugins are unloaded using:

delete require.cache[modulePath]

But if they are requires in the module main file, it will not be unloaded.

So, when you update your module code, it will mix old and new modules.

Describe the expected behavior

Plugin imported files should be unloaded too.
I think this module can help: https://www.npmjs.com/package/decache

Additional information

  • PeerTube instance:
    • version: 3.1.0
    • NodeJS version: 12
@JohnXLivingston
Copy link
Contributor Author

I can confirm that this workaround is working with my plugin:
JohnXLivingston/peertube-plugin-livechat@0bad31d

Without it, changes in my settings names are not visible unless I restart the server. With the call to decache, everything works fine.

@Chocobozzz Chocobozzz self-assigned this Apr 8, 2021
@Chocobozzz Chocobozzz added Component: PeerTube Plugin 📦 Features that can be developed in a plugin, but require PeerTube plugin API development Type: Bug 🐛 Confirmed bug, at least replicated once by another contributor labels Apr 8, 2021
@JohnXLivingston
Copy link
Contributor Author

@Chocobozzz , tell me if you want me to make a PR with a fix.

@Chocobozzz
Copy link
Owner

tell me if you want me to make a PR with a fix.

Yes please 👍

@JohnXLivingston
Copy link
Contributor Author

For the record, I found a similar potential bug in server/initializers, function reloadConfig.
Have you ever had a cache bug when reloading config? Maybe we should apply the same fix?

@Chocobozzz
Copy link
Owner

Chocobozzz commented Apr 9, 2021

Have you ever had a cache bug when reloading config?

No we haven't, but it safer to use decache here too

@JohnXLivingston
Copy link
Contributor Author

Ok, i'll do the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PeerTube Plugin 📦 Features that can be developed in a plugin, but require PeerTube plugin API development Type: Bug 🐛 Confirmed bug, at least replicated once by another contributor
Projects
None yet
Development

No branches or pull requests

2 participants