Skip to content
This repository has been archived by the owner on Dec 3, 2018. It is now read-only.

Better handling of zombie node.exe #1167

Open
springmeyer opened this issue Feb 5, 2015 · 10 comments
Open

Better handling of zombie node.exe #1167

springmeyer opened this issue Feb 5, 2015 · 10 comments

Comments

@springmeyer
Copy link
Contributor

Quick braindump while I'm debugging on windows.

Currently if there are zombie mapbox-studio.exe running when you try to re-install this will block the installer from running: f3415c0.

  • Improvements: help users know they need to restart if they are blocked (or to stop zombies in task manager)

But node.exe zombies can still be running and installer will continue on. The result is that Mapbox studio will likely not be able to start because app.db cannot be compacted (because node.exe has a hold of it). Also the .mapbox-studio folder can't be manually deleted because the node.exe has ahold of it. So this will occur if you manually try to delete:

screen shot 2015-02-04 at 7 54 46 pm

I also found that app.db.compacted could get left behind when a process could not start. Then all subsequent tries to restart would fail until the app.db and app.db.compacted are delected (only tested deleting the whole .mapbox-studio folder).

Takehome for support: always recommend deleting .mapbox-studio folder if a user is experience windows startup trouble. And if it can't be deleted then restart the machine to dispatch the zombies or kill all node.exe and mapbox-studio.exe processes in the task manager.

@yhahn
Copy link
Member

yhahn commented Feb 5, 2015

Will give some thought to this -- seems like one of the main problems atm is that db compaction requires file deletion, something that is contentious when other processes have a handle on the file on windows. I'll think about ways to avoid deletetion when doing db compaction or maybe finding a different way of storing our db entirely to avoid the issue.

@wilhelmberg
Copy link
Contributor

I think compaction is not the only problem. node.exe/mapbox-studio.exe cannot be deleted/replaced if they are still running. Un-/Installer shouldn't continue anyway if files cannot be removed/added. Haven't tested this though.

kill all node.exe and mapbox-studio.exe processes in the task manager.

I think suggesting a reboot is a safer instruction.
People will have to think about what other programs are running on their machine and don't blindly kill all processes.
Like, I sometimes have other node.exes runnig, that are not started by Mapbox Studio, e.g. Visual Studio with Node.js Tools or when working on serveral different problems at the same time.

@springmeyer
Copy link
Contributor Author

I think suggesting a reboot is a safer instruction.

I agree, and have been recommending reboot. I only provide that detail for those that might understand it, but yeah, I'll stop :)

Like, I sometimes have other node.exes runnig, that are not started by Mapbox Studio,

So, I don't know why there is a node.exe that studio starts actually. Ideally we could have everything have a custom name. Perhaps the node.exe is the atom shell process? @BergWerkGIS do you think we could rename that one too by either renaming the binary or setting process.title?

@wilhelmberg
Copy link
Contributor

Yes, maybe renaming to mbx-node.exe/studio-node.exe would make things easier.
node.exe is the actual server process, see third screenshot.
I don't think a different process.title for sub processes will show up in the standard Windows Task Manager.


This is the Mapbox Studio 0.2.5 process tree on Win7:
image

On Win8.1:
image

Interesting that the process hierarchy is different on W7 and W8.1


These are the parameters of each process:

mapbox-studio-process-parameters

@yhahn
Copy link
Member

yhahn commented Feb 10, 2015

@springmeyer @BergWerkGIS if we rename our node binary we should do it across all 3 platforms to keep this code consolidated:

https://github.com/mapbox/mapbox-studio/blob/mb-pages/index-shell.js#L9

@springmeyer
Copy link
Contributor Author

Realizing that renaming node.exe only windows probably works if all addons' are recompiled against that re-named binary (which is not practical) - more details at nodejs/node#751.

@wilhelmberg
Copy link
Contributor

This issue is that the name of the library that compiled addons link against is fixed. For this reason it also has never been possible to rename node.exe on windows and have compiled addons still work.

Under these circumstances it doesn't make sense.
We would have to have a dedicated studio build and a public build of all native modules.

@springmeyer
Copy link
Contributor Author

Agree. So, we'll need to find a different way to identify a running node.exe that represents the studio server process.

@wilhelmberg
Copy link
Contributor

When the zombies attacked, where there just the node.exe's left, or also mapboxstudio.exe?

2 ideas:

  • check the parent process: like in the screenshot. Each process has information about its parent process. If parent is mbstudio, kill it. Or, better, kill process tree of parent
  • check path of process image:
    • \Program Files\mapbox-studio for x86 studio on x86 OS and x64 studio on x64
    • \Program Files (x86)\mapbox-studio for x86 studio on x64 OS

@springmeyer
Copy link
Contributor Author

Realizing that renaming node.exe only windows probably works if all addons' are recompiled against that re-named binary (which is not practical) - more details at nodejs/node#751.

Flagging that I just saw "Renaming atom.exe no longer breaks native modules." in the atom shell changelog: https://github.com/atom/atom-shell/releases/tag/v0.23.0. Not sure if that means this is fixed for node.exe or not upstream....

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants