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

Interactive Terminal / TTY Hangup #19

Closed
0x726d77 opened this Issue Oct 28, 2015 · 16 comments

Comments

Projects
None yet
4 participants
@0x726d77

When using tini with a TTY, interactive shell commands like python and irb hang the terminal. Either of the following Dockerfiles will reproduce the issue (python can be replaced with ruby, etc.):

FROM krallin/ubuntu-tini:trusty
RUN apt-get update && apt-get install -y --no-install-recommends python
FROM debian:8.2

RUN apt-get update && \
    apt-get install -y --no-install-recommends ca-certificates curl libc6 python && \
    curl -sSL "https://github.com/krallin/tini/releases/download/v0.8.0/tini" -o /usr/bin/tini && \
    chmod +x /usr/bin/tini && \
    rm -rf /var/lib/apt/lists/*

ENTRYPOINT ["/usr/bin/tini", "--"]
$ docker build -t tini-tty .
$ docker run --rm -it tini-tty python

Commenting out the ENTRYPOINT and rebuilding the image results in a working TTY session. Non-interactive commands, such as $ docker run --rm -it tini-tty python --version work just fine.

Am I missing something simple here like an option? Thanks!

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 28, 2015

Owner

Indeed, this isn't super surprising. Interactive mode isn't something I have considered as a goal so far (and thus not something I worked on).

Now, that doesn't mean it shouldn't be supported! If it's lightweight and doesn't make the tool harder to use, then it should be supported (one of the design goals of Tini is that adding it shouldn't break things that would otherwise work).

I'll try to get to the bottom of this and fix it (happy to hear suggestions!).

Cheers,

Owner

krallin commented Oct 28, 2015

Indeed, this isn't super surprising. Interactive mode isn't something I have considered as a goal so far (and thus not something I worked on).

Now, that doesn't mean it shouldn't be supported! If it's lightweight and doesn't make the tool harder to use, then it should be supported (one of the design goals of Tini is that adding it shouldn't break things that would otherwise work).

I'll try to get to the bottom of this and fix it (happy to hear suggestions!).

Cheers,

@krallin krallin self-assigned this Oct 28, 2015

@0x726d77

This comment has been minimized.

Show comment
Hide comment
@0x726d77

0x726d77 Oct 28, 2015

Thanks! This isn't a mission-critical requirement... I just wondered if I was missing something obvious. FWIW, here's the output of running w/ -vvv:

$ docker run --rm -it tini python
[INFO ] Spawned child process 'python' with pid '8'
Python 2.7.6 (default, Jun 22 2015, 17:58:13)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
[DEBUG] Passing signal: 'Window changed'
[TRACE] No child to reap
[DEBUG] Received SIGCHLD
[TRACE] No child to reap
[...]

Thanks! This isn't a mission-critical requirement... I just wondered if I was missing something obvious. FWIW, here's the output of running w/ -vvv:

$ docker run --rm -it tini python
[INFO ] Spawned child process 'python' with pid '8'
Python 2.7.6 (default, Jun 22 2015, 17:58:13)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
[DEBUG] Passing signal: 'Window changed'
[TRACE] No child to reap
[DEBUG] Received SIGCHLD
[TRACE] No child to reap
[...]
@danilobuerger

This comment has been minimized.

Show comment
Hide comment
@danilobuerger

danilobuerger Oct 28, 2015

Contributor

@mattwilliamsnyc just as a sidenote: you should check the hash of tini when curling it from github. See https://github.com/jenkinsci/docker/blob/master/Dockerfile for an example.

Contributor

danilobuerger commented Oct 28, 2015

@mattwilliamsnyc just as a sidenote: you should check the hash of tini when curling it from github. See https://github.com/jenkinsci/docker/blob/master/Dockerfile for an example.

@0x726d77

This comment has been minimized.

Show comment
Hide comment
@0x726d77

0x726d77 Oct 28, 2015

@danilobuerger Thanks! That example is not how I'm installing tini (or python or anything else :))... I was just trying to keep it small and to the point... but out of curiosity, where did you get the checksum?

@danilobuerger Thanks! That example is not how I'm installing tini (or python or anything else :))... I was just trying to keep it small and to the point... but out of curiosity, where did you get the checksum?

@danilobuerger

This comment has been minimized.

Show comment
Hide comment
@danilobuerger

danilobuerger Oct 28, 2015

Contributor

The first time you could just download it and do $ openssl dgst -sha1 tini

Contributor

danilobuerger commented Oct 28, 2015

The first time you could just download it and do $ openssl dgst -sha1 tini

@0x726d77

This comment has been minimized.

Show comment
Hide comment
@0x726d77

0x726d77 Oct 28, 2015

Ah, ok. I didn't know if this was published somewhere as an .asc or whatever. I have an update script that scrapes for the latest version and updates my base Dockerfiles:

curl -s https://github.com/krallin/tini/releases/latest | grep -o "/v.*\"" | sed 's:^..\(.*\).$:\1:'

I can do the hashing there and pin that as well. Thanks for the tip!

Ah, ok. I didn't know if this was published somewhere as an .asc or whatever. I have an update script that scrapes for the latest version and updates my base Dockerfiles:

curl -s https://github.com/krallin/tini/releases/latest | grep -o "/v.*\"" | sed 's:^..\(.*\).$:\1:'

I can do the hashing there and pin that as well. Thanks for the tip!

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 28, 2015

Owner

I eventually want to make those hashes published somewhere, or use signatures (but right now the builds happen in Travis so it's not entirely trivial!)

Owner

krallin commented Oct 28, 2015

I eventually want to make those hashes published somewhere, or use signatures (but right now the builds happen in Travis so it's not entirely trivial!)

krallin added a commit that referenced this issue Oct 28, 2015

Make child process group foreground on active tty
When a tty is available, make the child process group foreground on
(and graciously fallback if no tty is available).

Fix #19
@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 28, 2015

Owner

Hey @mattwilliamsnyc,

I was able to reproduce the issue, and I think I have a fix for it in #21

We introduced this as a side effect of #15 (tests didn't catch it, but now we have some that will).

The problem is that the foreground process group on the tty is Tini, not the child process group, which thus means the child process (Python in your example) hangs when it tries to write to tty / set settings.

In Python's case, this happens when the readline module is loaded (you can see that by running python -vvv). Python gets interrupted by SIGTTOU, which is also why tini receives a SIGCHLD with nothing to reap in the log you sent over.


The solution in #21 is to have Tini make the child process group the new foreground process group (and to gracefully bail if there is no tty).

I want to test a few last things before I merge it (namely identify whether there's a scenario where Tini itself ends up getting interrupted by SIGTTOU when writing to stdout after backgorunding), but I think I'll do that soon.

Cheers,

Owner

krallin commented Oct 28, 2015

Hey @mattwilliamsnyc,

I was able to reproduce the issue, and I think I have a fix for it in #21

We introduced this as a side effect of #15 (tests didn't catch it, but now we have some that will).

The problem is that the foreground process group on the tty is Tini, not the child process group, which thus means the child process (Python in your example) hangs when it tries to write to tty / set settings.

In Python's case, this happens when the readline module is loaded (you can see that by running python -vvv). Python gets interrupted by SIGTTOU, which is also why tini receives a SIGCHLD with nothing to reap in the log you sent over.


The solution in #21 is to have Tini make the child process group the new foreground process group (and to gracefully bail if there is no tty).

I want to test a few last things before I merge it (namely identify whether there's a scenario where Tini itself ends up getting interrupted by SIGTTOU when writing to stdout after backgorunding), but I think I'll do that soon.

Cheers,

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 28, 2015

Owner

Oh, and I was about to forget: thanks a ton for the report!

Owner

krallin commented Oct 28, 2015

Oh, and I was about to forget: thanks a ton for the report!

@0x726d77

This comment has been minimized.

Show comment
Hide comment
@0x726d77

0x726d77 Oct 29, 2015

Hey @krallin,

Thanks for the explanation and the quick fix. I'll continue to report anything I encounter that doesn't match standard Docker behavior since that's a design goal.

Thanks again for the update and for all your work on this project!

Hey @krallin,

Thanks for the explanation and the quick fix. I'll continue to report anything I encounter that doesn't match standard Docker behavior since that's a design goal.

Thanks again for the update and for all your work on this project!

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 29, 2015

Owner

All right, I fixed the concerns I had yesterday. Will probably merge and release a new version tomorrow!

Owner

krallin commented Oct 29, 2015

All right, I fixed the concerns I had yesterday. Will probably merge and release a new version tomorrow!

krallin added a commit that referenced this issue Oct 31, 2015

Make child process group foreground on active tty
When a tty is available, make the child process group foreground on
(and graciously fallback if no tty is available).

Fix #19

@krallin krallin closed this in #21 Oct 31, 2015

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 31, 2015

Owner

@mattwilliamsnyc This should be fixed as of 0.8.2. I'm building new images and they should be live soon.

@danilobuerger since you're already in this thread ;). How do we get the Alpine packages updated to this new version? : ) Happy to help if I can.

Owner

krallin commented Oct 31, 2015

@mattwilliamsnyc This should be fixed as of 0.8.2. I'm building new images and they should be live soon.

@danilobuerger since you're already in this thread ;). How do we get the Alpine packages updated to this new version? : ) Happy to help if I can.

@danilobuerger

This comment has been minimized.

Show comment
Hide comment
@danilobuerger

danilobuerger Oct 31, 2015

Contributor

I dont mind to carry on maintaining this package for alpine. So best way for the future would be if you ping me here or via twitter when you want it updated.

Contributor

danilobuerger commented Oct 31, 2015

I dont mind to carry on maintaining this package for alpine. So best way for the future would be if you ping me here or via twitter when you want it updated.

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin Oct 31, 2015

Owner

Awesome; thanks @danilobuerger. I'll continue pinging you in issues when a release goes out then! : )

Owner

krallin commented Oct 31, 2015

Awesome; thanks @danilobuerger. I'll continue pinging you in issues when a release goes out then! : )

@jamshid

This comment has been minimized.

Show comment
Hide comment
@jamshid

jamshid May 20, 2016

Thanks for interactive / tty support! I was using dumb-init but had to switch because it doesn't support it (Yelp/dumb-init#78).

jamshid commented May 20, 2016

Thanks for interactive / tty support! I was using dumb-init but had to switch because it doesn't support it (Yelp/dumb-init#78).

@krallin

This comment has been minimized.

Show comment
Hide comment
@krallin

krallin May 20, 2016

Owner

Thanks for your feedback @jamshid!

Owner

krallin commented May 20, 2016

Thanks for your feedback @jamshid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment