volumio / Volumio2

Volumio 2 - Audiophile Music Player
http://volumio.org
Other
1.37k stars 315 forks source link

Plugin manager: some erratic installs #847

Closed macmpi closed 6 years ago

macmpi commented 7 years ago

Hi,

I encountered erratic plugin installs several times, particularly when installing a dropped zip in install UI. Indeed it seems that sometimes installation may happen twice (the second would fail if the first one was successfully registered). It may be due to insufficient clean-up of temporary install files upon installation failure. If some install procedure fails, plugin manager may discover remainings of /tmp/downloaded_plugin.zip and try to install over-again (an exemple here).

Maybe it would be suitable to remove each temporary file & folders after each step even if not successful (/tmp/downloaded_plugin.zip, /tmp/downloadedPlugin folder, and /tmp/plugins folder) or find a way to avoid those multiple installs.

volumio commented 7 years ago

Yes, I would say we should understand why installation restarts. Any idea?

macmpi commented 7 years ago

I'm afraid no idea at this time. However /tmp cleanup would at least avoid consequences, while freeing-up some RAM in the meantime. Re-entry could still be noticed with journalctl messages, so problem does not go unnoticed.

macmpi commented 7 years ago

Isn't the issue with this, which may be called several times in pages refreshes?

After first transfer to /tmp/plugins/ and install upon socket.emit('installPlugin', { url:pluginurl});, installPlugin does not delete original file from /tmp/plugins/after install. Therefore any re-entry in app.route('/plugin-upload') will produce re-install.

macmpi commented 7 years ago

While at it, it would be nice if RAM usage can be optimized by limiting plugin duplication in /tmp: indeed installing some big plugins (like spotifyconnect for arm6) on limited RAM devices (such as rpi0) may lead to fairly low RAM conditions. Here are some suggestions, but you may have better ideas.

For drag&drop install, can't the initial upload be directly into /tmp/downloaded_plugin.zip (omitting temporary /tmp/plugin/myplugin.zip), and then just test inside installPlugin() from pluginmanager.js, if that file already exists (or empty URL passed) to differentiate drag&drop or network install (and do wget only in the latter case)? Then, can /tmp/downloaded_plugin.zip extraction happen directly into /data/plugins/tmp_plugin (and delete /tmp/downloaded_plugin.zip right-away), and finally move tmp_plugin into final destination with final name once package.json is actually read (and no error found)?

Those (or similar) measures could significantly save RAM in some cases, and by cleaning after each step, would also be safer, while the re-entry issue is being investigated in drag&drop situations. Thanks for consideration.

volumio commented 7 years ago

This is really a good idea. Give e some time to implement it

simonspa commented 7 years ago

I'm having the same issue with this. Especially when the installation of the plugin takes a while (calling sudo apt-get update in a RPi takes quite long), a second instance is spawned during the installation process...

balbuze commented 7 years ago

Yes, volumio 2.041 problem is still here. If plugin needs to download a file, we can see several download (each time the plugin installation restarts, a new download starts). Even aif at the end the installation works, it takes twice the time, and cpu to unzip the file.

xipmix commented 7 years ago

The double-install seems to be some kind of failure to clear state in the UI.

My perception, which could well be wrong is as follows. On a linux box with xfce & firefox, I can make it do two uploads by performing one, and then clicking again with my left mouse button on a tab in the browser or even in a terminal window. It seems to be that somehow the UI picks up the event and reruns the upload. I am always doing the upload by clicking in the box and selecting a file from the 'file open' popup, not dragging and dropping. I have managed to make it happen more than once too but I am not really sure how.

balbuze commented 7 years ago

I'm always doing like you do. Clicking the file. Firefox on xubuntu. And sometimes it starts twice. Even if the plugin install fails, if no action (closing the dialogue), after a while it restarts...

xipmix commented 7 years ago

@macmpi I think I can see a way to implement direct reuse of the file the user uploaded, which will save some space in /tmp. The patch depends on changes made in #1083 so a PR will have to wait for volumio to have some time to consider & perhaps merge that.

macmpi commented 7 years ago

nice! Any clue on the "reload twice" thing? BTW I've not checked if the same issue happens when uploading backgrounds from Appearance panel: might give clues. (I remember once, I had a plugin "magically" land into the background folder!...)

xipmix commented 7 years ago

None. It will require watching with a live debugger. The point about backgrounds is interesting though; might be easier to catch that in the act. I had a case of a double-upload happening while I was on a different xfce desktop. So it might be some kind of timeout, rather than any UI events I am generating.

macmpi commented 7 years ago

Looking at it more closely, while working on #1220 It turns-out initial upload happen twice: for each dragged & dropped plugin, 2 files are generated in /tmp/plugins So following that, plugin install logically happens twice.

So we "just" need to find-out why drag & drop upload happens twice originally... Does not seem a pluginmanager issue then...

xipmix commented 7 years ago

In #1105 I noticed the plugin is uploaded by the user, then reuploaded to the express server on port 3000. Are you saying that the d&d method does the first upload twice?

macmpi commented 7 years ago

Sorry my bad, I was misinterpreting something across several tests.

macmpi commented 7 years ago

~~interestingly with https://github.com/volumio/Volumio2/pull/1231, duplicate installs (per say) seem fixed: this is probably due to more aggressive temporary files cleanup. However, dialog message still reports install twice: so the underlying "drop zone" issue is still there, while the most annoying consequence is fixed.~~ Maybe this could help (last note)?

macmpi commented 7 years ago

issue is still here as of v2.278. Web UI does call pluginmanager twice in a 2sec intervall:

Sep 21 11:32:19 Volumio volumio[860]: Uploading: cdio_paranoia.zip
Sep 21 11:32:19 Volumio volumio[860]: Created safe filename as '4cef67b8-b26f-4e4b-bd50-55cca9efa8a9.zip'
Sep 21 11:32:20 Volumio volumio[860]: Upload Finished of cdio_paranoia.zip as 4cef67b8-b26f-4e4b-bd50-55cca9efa8a9.zip
Sep 21 11:32:20 Volumio volumio[860]: info: Downloading plugin at http://127.0.0.1:3000/plugin-serve/4cef67b8-b26f-4e4b-bd50-55cca9efa8a9.zip
Sep 21 11:32:20 Volumio volumio[860]: info: END DOWNLOAD: http://127.0.0.1:3000/plugin-serve/4cef67b8-b26f-4e4b-bd50-55cca9efa8a9.zip

Sep 21 11:34:20 Volumio volumio[860]: Uploading: cdio_paranoia.zip
Sep 21 11:34:20 Volumio volumio[860]: Created safe filename as '40608650-7660-4db4-94b4-d583810a4afd.zip'
Sep 21 11:34:26 Volumio volumio[860]: info: Fetched 17.9 MB in 1min 24s (212 kB/s)
Sep 21 11:34:26 Volumio volumio[860]: Upload Finished of cdio_paranoia.zip as 40608650-7660-4db4-94b4-d583810a4afd.zip
Sep 21 11:34:26 Volumio volumio[860]: info: Downloading plugin at http://127.0.0.1:3000/plugin-serve/40608650-7660-4db4-94b4-d583810a4afd.zip
Sep 21 11:34:27 Volumio volumio[860]: info: END DOWNLOAD: http://127.0.0.1:3000/plugin-serve/40608650-7660-4db4-94b4-d583810a4afd.zip

Happened same with either drag&drop of click-download from macOS/Chromium/FF (could be platform/browser dependant maybe).

balbuze commented 7 years ago

Yes I can confirm the behaviour. And other point is that if you install a plugin when an other plugin is installed, at the end of the install, the list shows a new line but the plugin name is the other plugin. A page refresh solve this.

macmpi commented 6 years ago

I guess this is not happening again since local upload/dropbox UI has been removed in favor of volumio plugin install manual command... Will close, but @balbuze @xipmix let me know if you still see it.

volumio commented 6 years ago

Fixed