Skip to content

Commit

Permalink
daemon: don't use Popen on linux/mac (#4262)
Browse files Browse the repository at this point in the history
We used `Popen` just to provide a general way of launching our daemons
on both Windows and *nix, but it is causing us issues when dvc is
launched from some exotic places (like nix package) or through `main()`
helper from a script that is not dvc.

This patch makes us just use `main()` directly in our double-forked
process.

Windows implementation has to use the `Popen` mechanism, because this is
the only way to daemonize a process on Windows. This means that dvc on
Windows is still susceptible to this problem, but there is nothing we
can do about it right now :(

Fixes #4206
Fixes #4165
  • Loading branch information
efiop authored Jul 23, 2020
1 parent d346b8e commit f799e1e
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions dvc/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@
def _spawn_windows(cmd, env):
from subprocess import STARTUPINFO, STARTF_USESHOWWINDOW

prefix = [sys.executable]
if not is_binary():
prefix += [sys.argv[0]]

creationflags = CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS

startupinfo = STARTUPINFO()
startupinfo.dwFlags |= STARTF_USESHOWWINDOW

Popen(
cmd,
prefix + cmd,
env=env,
close_fds=True,
shell=False,
Expand All @@ -34,6 +38,8 @@ def _spawn_windows(cmd, env):


def _spawn_posix(cmd, env):
from dvc.main import main

# NOTE: using os._exit instead of sys.exit, because dvc built
# with PyInstaller has trouble with SystemExit exception and throws
# errors such as "[26338] Failed to execute script __main__"
Expand All @@ -59,7 +65,8 @@ def _spawn_posix(cmd, env):
sys.stdout.close()
sys.stderr.close()

Popen(cmd, env=env, close_fds=True, shell=False).communicate()
os.environ.update(env)
main(cmd)

os._exit(0) # pylint: disable=protected-access

Expand Down Expand Up @@ -87,10 +94,7 @@ def daemon(args):
logger.debug("skipping launching a new daemon.")
return

cmd = [sys.executable]
if not is_binary():
cmd += [sys.argv[0]]
cmd += ["daemon", "-q"] + args
cmd = ["daemon", "-q"] + args

env = fix_env()
file_path = os.path.abspath(inspect.stack()[0][1])
Expand Down

0 comments on commit f799e1e

Please sign in to comment.