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

Killing the shim process does not kill the child process #3294

Closed
ogerardin opened this issue Apr 1, 2019 · 14 comments
Closed

Killing the shim process does not kill the child process #3294

ogerardin opened this issue Apr 1, 2019 · 14 comments

Comments

@ogerardin
Copy link
Contributor

ogerardin commented Apr 1, 2019

This issue originates from a problem with dbtest from project mgo (Mongo driver for Go): globalsign/mgo#328. I believe this issue is a generic issue with scoop, which is exemplified with mongod+dbtest.

Observed behaviour
When mongod is installed using scoop, a shallow executable called a "shim" is created in the path (usually in %USERPROFILE%\scoop\shims). The actual mongod.exe lies somewhere else (normally %USERPROFILE%\scoop\apps\mongodb\current).
When you launch mongod from the command line, the shim launches the actual mongod as a child process. The same happens when dbtest launches mongod, but when it tries to stop the server only the shim gets terminated, the actual server process stays alive and is adopted by userinit.

Expected behaviour
Killing the shim should kill the spawned process.

@ogerardin
Copy link
Contributor Author

I believe this is the same bug as exposed in this issue: #1896

@ogerardin ogerardin changed the title Killing the shim process does not kill the process tree Killing the shim process does not kill the child process Apr 1, 2019
@ogerardin
Copy link
Contributor Author

Also probably related: #2339

@71
Copy link

71 commented Apr 7, 2019

To deal with these problems I made my own shim. Maybe you could try it out, and if it works in your case as well, we could move to see if the official shim could be replaced by my implementation?

@ogerardin
Copy link
Contributor Author

To deal with these problems I made my own shim. Maybe you could try it out, and if it works in your case as well, we could move to see if the official shim could be replaced by my implementation?

Nice! Unfortunately I do not have Visual Studio, I'll try to compile it with gcc and test...

@71
Copy link

71 commented Apr 8, 2019

Ah, that's too bad. I'm not sure gcc can compile it though, since windows.h / SHELL32.LIB are used. There is a pre-compiled binary in the releases, but I absolutely understand if you don't want to run unknown binaries on your machine.

@ogerardin
Copy link
Contributor Author

I managed to compile it with MingW/gcc, it was just missing #define ERROR_ELEVATION_REQUIRED.
However, it seems it does not pass the command-line arguments to the target executable. To be sure I tried also with the released version that you made, and it has the same behaviour...

@71
Copy link

71 commented Apr 8, 2019

Ah, I'll try to fix this asap then, and will let you know when updates are available.

Update: Did some debugging and my arguments / standard input are passed without a problem. I did find a bug, but it was unrelated.

@ogerardin
Copy link
Contributor Author

When I use your shim.exe and start mongod with "--port 12345" for example, any arguments such as --port are ignored and it listens on port 27017 (the default). Tried with Windows 10 64 bits.
What did you try it with, so I can see if it works for me?

@71
Copy link

71 commented Apr 9, 2019

I tried executing fzf.exe both with and without default command line arguments (specified in fzf.shim), and for me everything worked, including passing arguments and a string via the standard input. Windows 10 64bits as well.

@ogerardin
Copy link
Contributor Author

ogerardin commented Apr 9, 2019

I tried again (took your shim.exe from the release and ran repshims.bat) and this time the arguments passing works. I must have done something wrong...
However, my test still fails: killing the shim does not kill the child.

If you want to test by yourself:

scoop install mongodb
scoop install go
go get github.com/globalsign/mgo
go test github.com/globalsign/mgo/dbtest

This results in :

FAIL: dbserver_test.go:58: S.TestStop

dbserver_test.go:83:
c.Fatalf("Stop did not stop the server")
... Error: Stop did not stop the server

If you manage to make the test pass with your shim then I say we have a good candidate to replace the native shim :)

@71
Copy link

71 commented Apr 9, 2019

@ogerardin Alright, the last commit should make this work! I now create a job object which instructs Windows to kill our child when the parent dies, so I don't even have to manage this manually.

@ogerardin
Copy link
Contributor Author

Killing the parent manually now does effectively kill the child !
Unfortunately it still doesn't work with mongod and the Go package, but I don't know why...

BTW I compiled it with gcc and the exe is only 47K.

@ogerardin
Copy link
Contributor Author

I created a pull request in your repo for gcc: 71/scoop-better-shimexe#1

@rasa
Copy link
Member

rasa commented Dec 23, 2020

This issue should be fixed in #3998, which was merged into master. To use, type:

scoop config shim kiennq
scoop reset *

or

scoop config shim 71
scoop reset *

Eventually, one of those shims (probably kiennq) will be made the default.

If this issue is still a problem after trying both solutions above, please reopen this issue. Thanks!

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

No branches or pull requests

3 participants