Skip to content

Commit

Permalink
Do not use setsid directly, use dedicated start_new_session
Browse files Browse the repository at this point in the history
Just ran into
https://stackoverflow.com/questions/42257512/difference-between-subprocess-popen-preexec-fn-and-start-new-session-in-python
which points to python docs at
https://docs.python.org/3.4/library/subprocess.html?highlight=subprocess#popen-constructor

    Warning The preexec_fn parameter is not safe to use in the presence
    of threads in your application. The child process could deadlock before
    exec is called. If you must use it, keep it trivial! Minimize the number of
    libraries you call into.

    Note If you need to modify the environment for the child use the env
    parameter rather than doing it in a preexec_fn. The start_new_session
    parameter can take the place of a previously common use of preexec_fn to
    call os.setsid() in the child.

So smells like we better use this dedicated option added in 3.2 .
  • Loading branch information
yarikoptic committed Aug 21, 2024
1 parent ebf7b17 commit d02d2ec
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/con_duct/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ def execute(args: Arguments) -> int:
[str(args.command)] + args.command_args,
stdout=stdout_file,
stderr=stderr_file,
preexec_fn=os.setsid,
start_new_session=True,
)
except FileNotFoundError:
# We failed to execute due to file not found in PATH
Expand Down

0 comments on commit d02d2ec

Please sign in to comment.