From b8f99e27e13658fd1e33c0e677f657514abc6538 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Mon, 25 Apr 2016 10:58:16 -0700 Subject: journal: fix already offline check and thread leak (#2810) Early in journal_file_set_offline() f->header->state is tested to see if it's != STATE_ONLINE, and since there's no need to do anything if the journal isn't online, the function simply returned here. Since moving part of the offlining process to a separate thread, there are two problems here: 1. We can't simply check f->header->state, because if there is an offline thread active it may modify f->header->state. 2. Even if the journal is deemed offline, the thread responsible may still need joining, so a bare return may leak the thread's resources like its stack. To address #1, the helper journal_file_is_offlining() is called prior to accessing f->header->state. If journal_file_is_offlining() returns true, f->header->state isn't even checked, because an offlining journal is obviously online, and we'll just continue with the normal set offline code path. If journal_file_is_offlining() returns false, then it's safe to check f->header->state, because the offline_state is beyond the point of modifying f->header->state, and there's a memory barrier in the helper. If we find f->header->state is != STATE_ONLINE, then we call the idempotent journal_file_set_offline_thread_join() on the way out of the function, to join a potential lingering offline thread. --- src/journal/journal-file.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index bed825cdc3..12902d9f91 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -217,8 +217,10 @@ int journal_file_set_offline(JournalFile *f, bool wait) { if (!(f->fd >= 0 && f->header)) return -EINVAL; - if (f->header->state != STATE_ONLINE) - return 0; + /* An offlining journal is implicitly online and may modify f->header->state, + * we must also join any potentially lingering offline thread when not online. */ + if (!journal_file_is_offlining(f) && f->header->state != STATE_ONLINE) + return journal_file_set_offline_thread_join(f); /* Restart an in-flight offline thread and wait if needed, or join a lingering done one. */ restarted = journal_file_set_offline_try_restart(f); -- cgit v1.2.3-54-g00ecf