Handle descriptor in non-blocking mode properly.#551
Conversation
|
LGTM, @giuseppe would you PTAL |
giuseppe
left a comment
There was a problem hiding this comment.
I've left some comments.
Would it still work if we include only these chunks?
if (errno == EAGAIN || errno == EWOULDBLOCK) {
// Non-blocking mode - no data available, return gracefully
return false;
and:
if (!written)
return false;
src/ctr_stdio.c
Outdated
|
|
||
| if (poll_ret == 0) { | ||
| // No data available, return gracefully | ||
| return true; |
There was a problem hiding this comment.
if we keep looping in drain_stdio I don't see the advantage in using a non blocking socket. We need to return false here and do not attempt to read again or am I missing something?
| } else if (num_read < 0) { | ||
| if (errno == EAGAIN || errno == EWOULDBLOCK) { | ||
| // Non-blocking mode - no data available, return gracefully | ||
| return true; |
src/ctr_stdio.c
Outdated
|
|
||
| // Use poll to check if data is available before reading | ||
| struct pollfd pfd = {.fd = fd, .events = POLLIN}; | ||
| int poll_ret = poll(&pfd, 1, 500); // 500ms timeout |
There was a problem hiding this comment.
this will slow down conmon cleanup if there is no data available and I don't see why we need to poll the fd? If we end up here, there was an event of the file descriptor or we are draining the fd until there is data, but in this case the fd is in non-blocking mode. Do we get spurious events that cause read_stdio to be called without any data avaiable?
|
Ok, let's keep it simple - I will remove the polling - it adds more complexity and potentially could slow things down (+the 500ms magic constant). |
Resolves: containers#490 Signed-off-by: Jindrich Novy <jnovy@redhat.com>
Resolves: #490