toolshed/docker-cli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix a race in cleaning up after journald followers

Browse Source 
When following a journal-based log, it was possible for the worker
goroutine, which reads the journal using the journal context and sends
entry data down the message channel, to be scheduled after the function
which started it had returned.  This could create problems, since the
invoking function was closing the journal context object and message
channel before it returned, which could trigger use-after-free segfaults
and write-to-closed-channel panics in the worker goroutine.

Make the cleanup in the invoking function conditional so that it's only
done when we're not following the logs, and if we are, that it's left to
the worker goroutine to close them.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind)
Upstream-commit: 52c0f36f7b7aa794932fa41dfe50dc85f78e6146
Component: engine
This commit is contained in:
Nalin Dahyabhai
2016-03-17 16:19:54 -04:00
parent ecc2f43e0e
commit a49b1e8f69
1 changed files with 17 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
components/engine/daemon/logger/journald/read.go
View File

@ -186,6 +186,8 @@ func (s *journald) followJournal(logWatcher *logger.LogWatcher, config logger.Re
s.readers.mu.Lock()
delete(s.readers.readers, logWatcher)
s.readers.mu.Unlock()
C.sd_journal_close(j)
close(logWatcher.Msg)
}()
// Wait until we're told to stop.
select {
@ -203,14 +205,24 @@ func (s *journald) readLogs(logWatcher *logger.LogWatcher, config logger.ReadCon
var pipes [2]C.int
cursor := ""
defer close(logWatcher.Msg)
// Get a handle to the journal.
rc := C.sd_journal_open(&j, C.int(0))
if rc != 0 {
logWatcher.Err <- fmt.Errorf("error opening journal")
close(logWatcher.Msg)
return
}
defer C.sd_journal_close(j)
// If we end up following the log, we can set the journal context
// pointer and the channel pointer to nil so that we won't close them
// here, potentially while the goroutine that uses them is still
// running. Otherwise, close them when we return from this function.
following := false
defer func(pfollowing *bool) {
if !*pfollowing {
C.sd_journal_close(j)
close(logWatcher.Msg)
}
}(&following)
// Remove limits on the size of data items that we'll retrieve.
rc = C.sd_journal_set_data_threshold(j, C.size_t(0))
if rc != 0 {
@ -286,6 +298,9 @@ func (s *journald) readLogs(logWatcher *logger.LogWatcher, config logger.ReadCon
logWatcher.Err <- fmt.Errorf("error opening journald close notification pipe")
} else {
s.followJournal(logWatcher, config, j, pipes, cursor)
// Let followJournal handle freeing the journal context
// object and closing the channel.
following = true
}
}
return