Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`stack exec` not propagating signals when using Docker #547

Closed
nh2 opened this issue Jul 8, 2015 · 5 comments
Closed

`stack exec` not propagating signals when using Docker #547

nh2 opened this issue Jul 8, 2015 · 5 comments
Assignees
Milestone

Comments

@nh2
Copy link
Collaborator

@nh2 nh2 commented Jul 8, 2015

This is exactly the same issue as #527, but when you have a stack.yaml that is Docker-enabled:

docker:
  enable: true

I falsely assumed that #527 was also about the Docker case, so please refer to #527 (comment) for a description of the problem.

This is the Docker issue that I believe would resolve this problem automatically if implemented: moby/moby#7567

@borsboom
Copy link
Contributor

@borsboom borsboom commented Jul 11, 2015

Not holding my breath on the upstream Docker issue getting fixed. Solution is probably to have stack catch signals and then proxy those itself using docker kill -s <signal>, at least for sigTERM and maybe some others.

@snoyberg snoyberg modified the milestone: 0.3.0.0 Jul 13, 2015
@borsboom borsboom modified the milestones: 0.2.0.0, 0.3.0.0 Jul 22, 2015
borsboom added a commit that referenced this issue Jul 28, 2015
borsboom added a commit that referenced this issue Jul 28, 2015
@borsboom
Copy link
Contributor

@borsboom borsboom commented Jul 29, 2015

I've improved this significantly, and it at least works for sigTERM now. The container's current PID 1 (phusion/baseimage's my_init) doesn't pass other signals along to subprocesses, so I also have a workaround that, in non-interactive contexts, proxies a sigINT received by stack to a sigTERM for the container (this makes stopping a Bamboo/Jenkins task work properly, but keeps Ctrl-C working normally for other cases).

To fully fix this, we'd need a different PID 1. One approach would be to have stack itself act as PID 1, and have it pass along signals to its subprocesses, as well as reap zombie processes.

@nh2 Can you try the current master out for your use case?

@borsboom
Copy link
Contributor

@borsboom borsboom commented Sep 6, 2015

Blocked by #531.

@borsboom borsboom modified the milestones: Docker, 0.2.0.0 Sep 6, 2015
@borsboom borsboom modified the milestones: Docker, P2 Sep 12, 2015
@borsboom
Copy link
Contributor

@borsboom borsboom commented Oct 15, 2015

  • A solution to this must handle the Ctrl-C in GHCI case mentioned in #1163.
@borsboom borsboom modified the milestones: P1: Must, P2: Should Oct 15, 2015
@borsboom borsboom removed the blocked label Nov 16, 2015
@borsboom borsboom closed this in 52944d2 Nov 30, 2015
@borsboom
Copy link
Contributor

@borsboom borsboom commented Nov 30, 2015

I've improved the signal handling significantly, but I don't think perfection is possible (I now understand why the Docker team decided to punt on this entirely for the interactive containers). More signals are now proxied, sigINT handling is more consistent, and sigTERM and sigABRT now give the container 30 seconds to exit gracefully before terminating it forcefully. Many signals don't make sense to proxy, and process control (STOP/CONT/etc.) doesn't work anyway (since there's usually no shell in the container to manage them, and if there is a shell it takes care of them on its own anyway), so these are not proxied. We can improve handling for signals on a case-by-case basis if a need can be demonstrated and there's a way to improve it without degrading other cases.

borsboom referenced this issue Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants