Skip to content

Commit

Permalink
Enhance dotenv run: Switch to execvpe for better resource management …
Browse files Browse the repository at this point in the history
…and signal handling (#523)

The current implementation of `dotenv run` CLI uses `subprocess.Popen`, which spawns a child process to execute the specified command. 

```
p = Popen(command,
              universal_newlines=True,
              bufsize=0,
              shell=False,
              env=cmd_env)
```

After spawning the child process, it exits with the same exit code returned by the child process.

```
ret = run_command(commandline, dotenv_as_dict)
exit(ret)
```

### We can enhance `dotenv run` usage dramatically while preserving exactly the same behaviour

By switching to `os.execvpe` instead of `subprocess.Popen`, we can replace the parent dotenv process with the new process specified by the user. This results in only one active process—the program the user intended to run.

**Benefits:**

1. No hanging parent process
    `dotenv run` acts as a launcher, so after executing `dotenv run redis-server`, only the Redis server process remains. The dotenv process, along with its Python interpreter, is completely replaced. This prevents the dotenv process from consuming RAM and other resources, which would otherwise persist until the Redis server exits.

2. Proper signal handling
    When using `subprocess.Popen`, the parent process (e.g., `dotenv`) remains responsible for handling and forwarding signals, which can lead to issues if the command doesn’t receive them directly. For instance, in Docker, if Redis was started without `exec`, it may not get important signals like `SIGTERM` when the container stops, potentially resulting in improper shutdowns or zombie processes. Using `os.execvpe` ensures that the command receives signals directly, improving reliability and making `dotenv` more suitable for production environments and improving reliability for DevOps engineers managing containerized applications.
    
All current logic will be preserved because dotenv run does not do anything special except propagate the child process exit code.

Thanks / @eekstunt
  • Loading branch information
eekstunt committed Jul 18, 2024
1 parent 08937a1 commit 4543837
Showing 1 changed file with 7 additions and 16 deletions.
23 changes: 7 additions & 16 deletions src/dotenv/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import shlex
import sys
from contextlib import contextmanager
from subprocess import Popen
from typing import Any, Dict, IO, Iterator, List

try:
Expand Down Expand Up @@ -161,14 +160,13 @@ def run(ctx: click.Context, override: bool, commandline: List[str]) -> None:
if not commandline:
click.echo('No command given.')
exit(1)
ret = run_command(commandline, dotenv_as_dict)
exit(ret)
run_command(commandline, dotenv_as_dict)


def run_command(command: List[str], env: Dict[str, str]) -> int:
"""Run command in sub process.
def run_command(command: List[str], env: Dict[str, str]) -> None:
"""Replace the current process with the specified command.
Runs the command in a sub process with the variables from `env`
Replaces the current process with the specified command and the variables from `env`
added in the current environment variables.
Parameters
Expand All @@ -180,20 +178,13 @@ def run_command(command: List[str], env: Dict[str, str]) -> int:
Returns
-------
int
The return code of the command
None
This function does not return any value. It replaces the current process with the new one.
"""
# copy the current environment variables and add the vales from
# `env`
cmd_env = os.environ.copy()
cmd_env.update(env)

p = Popen(command,
universal_newlines=True,
bufsize=0,
shell=False,
env=cmd_env)
_, _ = p.communicate()

return p.returncode
os.execvpe(command[0], args=command, env=cmd_env)

0 comments on commit 4543837

Please sign in to comment.