Use os.execvp for poetry run

[pbzweihander]

• Added tests for changed code.
• Updated documentation for changed code.

---

This resolves #993.

poetry run uses subprocess.call to run the other program, making poetry spawn a new child process.
The problem is when SIGTERM is handed over; poetry does not pass it to the child process, instead terminates itself and makes zombie processes.
So I changed poetry run to use os.execvp, so poetry will replace itself with a new process and properly handle signals.

Test

I made this small script in poetry/tester.py:

import signal
import time


def handler(signum, frame):
    print(f"signal {signum} received")


def main():
    print("start")
    signal.signal(signal.SIGTERM, handler)
    time.sleep(10)
    print("end")


if __name__ == "__main__":
    main()

Then ran poetry run python poetry/tester.py and tried to kill it.

Before

$ poetry run python poetry/tester.py &
[1] 23987
start
$ ps -ax | grep tester
24270 pts/11   S      0:00 python /home/.../poetry run python poetry/tester.py
24350 pts/11   S      0:00 /home/.../python poetry/tester.py
24359 pts/11   S+     0:00 grep --color=auto tester
$ kill -TERM $!
[1]+  Terminated              poetry run python poetry/tester.py
$ ps -ax | grep tester
24350 pts/11   S      0:00 /home/.../python poetry/tester.py
24446 pts/11   S+     0:00 grep --color=auto tester
$ 
end

Fixed

$ python -m poetry run python poetry/tester.py &
[1] 25282
start
$ ps -ax | grep tester
25282 pts/11   S      0:00 /home/.../python poetry/tester.py
25364 pts/11   S+     0:00 grep --color=auto tester
$ kill -TERM $!
signal 15 received
$ 
end
[1]+  Done                    python -m poetry run python poetry/tester.py

Test - scripts feature

I added the following lines to pyprojects.toml.

...
[tools.poetry.scripts]
...
tester = "poetry.tester:main"

Then ran poetry run tester and tried to kill it.

Before

$ poetry run tester &
[1] 27077
start
$ ps -ax | grep tester
27077 pts/11   S      0:00 python /home/.../poetry run tester
27173 pts/11   S      0:00 /home/.../python -c import sys; from importlib import import_module; sys.argv = ['tester']; import_module('poetry.tester').main()
27189 pts/11   S+     0:00 grep --color=auto tester
$ kill -TERM $!
[1]+  Terminated              poetry run tester
$ ps -ax | grep tester
27173 pts/11   S      0:00 /home/.../python -c import sys; from importlib import import_module; sys.argv = ['tester']; import_module('poetry.tester').main()
27266 pts/11   S+     0:00 grep --color=auto tester
$ 
end

Fixed

$ python -m poetry run tester &
[1] 27996
start
$ ps -ax | grep tester
27996 pts/11   S      0:00 /home/.../python -c import sys; from importlib import import_module; sys.argv = ['tester']; import_module('poetry.tester').main()
28082 pts/11   S+     0:00 grep --color=auto tester
$ kill -TERM $!
signal 15 received
$ 
end
[1]+  Done                    python -m poetry run tester

[digitalresistor]

👍 on this however using os.execvp does not work on Windows. Please take a look at and steal (my permission is hereby given to relicense it under whatever Poetry is licensed as) my code from here: https://github.com/bertjwregeer/vrun/tree/master/src/vrun which supports both Unix and Windows.

[digitalresistor]

specifically you want this file: https://github.com/bertjwregeer/vrun/blob/master/src/vrun/oscompat.py

[simnalamburt]

The document says that os.execvp is available in Windows. May I ask what exception is thrown when we try to use os.execvp in Windows?

https://docs.python.org/3/library/os.html#os.execvp

[pbzweihander]

I found in Windows, exec does not actually replace current process and instead creates a new one and exits the current one.

• Reference:
  • https://stackoverflow.com/questions/7004687/os-exec-on-windows
  • https://bugs.python.org/issue9148
  • https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2010/431x4c1w(v=vs.100)

So I think poetry needs to create a new process and wait until the new one exits. (https://github.com/bertjwregeer/vrun/blob/a726be8106a00621361f60079ac403e5bc16864a/src/vrun/oscompat.py#L34)

[digitalresistor]

If you try to use the exec calls on Windows you get into a really weird state where the process you are running can't be controlled from the cmd.exe window it is spawned from.

You need to do the whole popen dance so that you can send key strokes from the foreground to the process that was spawned on Windows.

[digitalresistor]

I see that @pbzweihander got there before me! :-D

Please steal my code, I didn't come to that conclusion the easy way!

[simnalamburt]

Now I think this PR is ready to be merged since I see no more blockers.

[brycedrennan]

Thanks for your contribution. I'm not sure when someone will have a chance to dig into this.

I imagine its pretty hard to add an automated test for this?

[digitalresistor]

What kind of tests are you looking for?

[maksbotan]

@sdispater so, are there plans to merge this one? :)

[sdispater]

Thanks a lot for this PR!

[rpdelaney]

Does this have any effect on performance?

[simnalamburt]

It'll boost performance since os.execvp is faster than subprocess.call

[akdor1154]

I think this broke calling scripts on Windows - if I do poetry run script.py it used to get called as subprocess.call(script.py, shell=True) but now gets called as Popen([script.py]) This doesn't work, see https://stackoverflow.com/questions/912830/using-subprocess-to-run-python-script-on-windows.

[simnalamburt]

It’s actually calling Popen([‘python3.exe’, blabla]) which seems OK according to your link https://stackoverflow.com/a/912847

[benjirewis]

I've noticed killing poetry processes with SIGTERM creates a status code of 143. While that error code does indicate graceful termination after receiving SIGTERM, why not just return 0?

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.