logs.go: fixed logger creation

[AntonMoryakov]

Previously, the done channel was not closed properly in case of an error returned by logPipe.Close(), potentially leading to a goroutine leak. This commit ensures that the done channel is closed even if there's an error during the closing of logPipe.

[kolyshkin]

LGTM

Wonder how have you found this @AntonMoryakov ?

[lifubang]

the done channel was not closed properly in case of an error returned by logPipe.Close()

I think we have catched the error returned by logPipe.Close(), so I think the done channel will be closed in any case.

runc/libcontainer/logs/logs.go

Lines 30 to 32 in 71524dc

	if err := logPipe.Close(); err != nil {
	logrus.Errorf("error closing log source: %v", err)
	}

Yes, but I think it will be better to move it to defer func. Thanks.

[kolyshkin]

the done channel was not closed properly in case of an error returned by logPipe.Close()

I think we have catched the error returned by logPipe.Close(), so I think the done channel will be closed in any case.

runc/libcontainer/logs/logs.go

Lines 30 to 32 in 71524dc

	if err := logPipe.Close(); err != nil {
	logrus.Errorf("error closing log source: %v", err)
	}

Hmm you are right (not sure how I overlooked it).

So, while defer is indeed better, there's nothing to fix here. For this reason, I'd rather not merge this.

[kolyshkin]

As pointed out above, there's nothing to fix here.

Or, at the very least, the description needs to be changed.

[kolyshkin]

@AntonMoryakov can you comment on #4254 (comment)?

[kolyshkin]

Closing for now; will reopen depending on the PR author response.