Skip to content

Conversation

@Almighty-Alpaca
Copy link

@Almighty-Alpaca Almighty-Alpaca commented Mar 9, 2025

Currently when stopping a Docker Compose stack using docker compose stop/down Docker simply waits 10 seconds (default timeout) and then kills the contains. This happens because Bash is running as PID 1 inside the container. Bash receives the stop signal, but doesn't forward it to Dawarich. When using exec to start Dawarich, Dawarich replaces the Bash process instead of being spawned as a child and therefore receives all signals directly. See krallin/tini#8 (comment) for a really good explanation on signals in Docker.

Additionally, I escaped all variables and removed useless assignments. Previously, unquoted variable usages could lead to unwanted shell expansion (although it shouldn't have been a problem before when using "sane" inputs for the variables).

@Almighty-Alpaca Almighty-Alpaca force-pushed the feature/docker-entrypoint-exec branch from eb81738 to 1ffd457 Compare March 9, 2025 20:35
@Almighty-Alpaca Almighty-Alpaca force-pushed the feature/docker-entrypoint-exec branch from 1ffd457 to ceb03c8 Compare May 19, 2025 18:52
@Freika Freika requested a review from Copilot June 5, 2025 19:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the Docker entrypoint scripts to use exec when launching processes so that signals are forwarded properly, while also improving variable quoting and removing unnecessary assignments.

  • Replaces Bash process with target processes via exec
  • Consistently quotes variable usage to avoid unintended shell expansion
  • Removes redundant environment variable assignments

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
docker/web-entrypoint.sh Improves quoting of DATABASE_URL extractions and uses exec for signal handling
docker/sidekiq-entrypoint.sh Similar quoting improvements with a minor inconsistency in quoting DATABASE_URL
Comments suppressed due to low confidence (1)

docker/sidekiq-entrypoint.sh:17

  • For consistency and to prevent any unexpected word splitting, consider quoting DATABASE_URL as in the other assignments, for example: DATABASE_NAME=$(echo "$DATABASE_URL" | awk -F[@/] '{print $5}').
DATABASE_NAME=$(echo $DATABASE_URL | awk -F[@/] '{print $5}')


# run passed commands
bundle exec ${@}
exec bundle exec "${@}"
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting the entire "${@}" may cause the arguments to be passed as a single string; consider using exec bundle exec "$@" to correctly separate multiple command arguments.

Suggested change
exec bundle exec "${@}"
exec bundle exec "$@"

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Almighty-Alpaca Almighty-Alpaca force-pushed the feature/docker-entrypoint-exec branch from ceb03c8 to 6e14217 Compare July 15, 2025 10:46
@Almighty-Alpaca Almighty-Alpaca force-pushed the feature/docker-entrypoint-exec branch from 6e14217 to 543242c Compare July 15, 2025 10:49
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

Successfully merging this pull request may close these issues.

1 participant