Resolve a set of race conditions in logging and attaching code#31856
Resolve a set of race conditions in logging and attaching code#31856tonistiigi merged 3 commits intomoby:masterfrom
Conversation
|
/cc @cpuguy83 |
|
Hi @cpuguy83 , any thoughts on this PR? |
|
Rebased. |
Signed-off-by: Jim Minter <jminter@redhat.com>
Signed-off-by: Jim Minter <jminter@redhat.com>
Signed-off-by: Jim Minter <jminter@redhat.com>
|
ping @tonistiigi... Can you take a look? |
|
ping @tonistiigi , any thoughts? |
| CloseStdin: container.Config.StdinOnce, | ||
| } | ||
| container.StreamConfig.AttachStreams(&cfg) | ||
| close(attached) |
There was a problem hiding this comment.
What's the purpose of this? afaics only some setup code runs before close, nothing that changes state.
There was a problem hiding this comment.
What's the issue behind this builder changes. I don't see it mentioned in the original issue. Also, why is this in Raw function that sets logs to false and not in regular ContainerAttach
There was a problem hiding this comment.
It is preventing a potential race where a build could get as far as outputting to std{out,err} before AttachStreams() had actually completed. In this case, any data outputted by the build would be lost. I didn't open a separate issue as it's very similar to #31323 (and #29285, which I fixed before); I was hoping just to fix it in passing as it's pretty straightforward.
|
LGTM |
When `docker logs` (client) exits, the daemon log contains "Error closing logger: invalid argument". This happens frequently when container doesn't run for long, the logDirver has already been closed when container exits before `ContainerLogs()` hits `cLog.Close()`. fix DTS2017062809604 backport from moby#31856 conflicts: daemon/attach.go daemon/logs.go Signed-off-by: Jim Minter <jminter@redhat.com> Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>
for long fix DTS2017062809604 partly backport from moby#31856 conflicts: daemon/logger/journald/read.go daemon/logger/jsonfilelog/read.go daemon/logs.go Signed-off-by: Jim Minter <jminter@redhat.com> Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>
Fix docker logs hang
Steps to reproduce the issue:
1. Run the attached script, eventually the `docker logs -f` command will hang:
```
#!/bin/bash
while true
do
container_id="$(docker run -d ubuntu:latest /bin/bash -e -c "echo -n \"foo\"")"
docker logs -f "${container_id}"
done
```
fix DTS2017062809604
backport from moby#31856
backport from moby#27782
backport from moby#22088
conflicts:
daemon/attach.go
daemon/logs.go
daemon/logger/journald/read.go
daemon/logger/jsonfilelog/read.go
daemon/logs.go
Signed-off-by: Jim Minter <jminter@redhat.com>
Signed-off-by: Tom Booth <tombooth@gmail.com>
Signed-off-by: Amit Krishnan <krish.amit@gmail.com>
Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>
See merge request docker/docker!625
Fixes #31323