msg: fix some locking issues

The wakeup_log_file callback was still assuming that mp_msg_lock was
used to control the log file thread, but this changed while I was
writing this code, and forgot to update it. (It doesn't change any
state, which is untypical for condition variable usage. The state that
is changed is protected by another lock instead. But log_file_lock still
needs to be acquired to ensure the signal isn't sent while the thread is
right before the pthread_cond_wait() call, when the lock is held, but
the signal would still be lost.)

Because the buffer's wakeup callback now acquires the lock, the wakeup
callback must be called outside of the buffer lock, to keep the lock
order (log_file_lock > mp_log_buffer.lock). Fortunately, the wakeup
callback is immutable, or we would have needed another dumb leaf lock.

mp_msg_has_log_file() made a similar outdated assumption. But now access
to the log_file field is much trickier; just define that it's only to be
called from the thread that manages the msg state. (The calling code
could also just check whether the log-file option changed instead, but
currently that would be slightly more messy.)
This commit is contained in:
wm4 2020-01-30 14:00:41 +01:00
parent 2fd34889fe
commit 355bb5b1e6
1 changed files with 8 additions and 7 deletions

View File

@ -318,6 +318,7 @@ static void write_msg_to_buffers(struct mp_log *log, int lev, char *text)
struct mp_log_root *root = log->root;
for (int n = 0; n < root->num_buffers; n++) {
struct mp_log_buffer *buffer = root->buffers[n];
bool wakeup = false;
pthread_mutex_lock(&buffer->lock);
int buffer_level = buffer->level;
if (buffer_level == MP_LOG_BUFFER_MSGL_TERM)
@ -358,9 +359,11 @@ static void write_msg_to_buffers(struct mp_log *log, int lev, char *text)
buffer->entries[pos] = entry;
buffer->num_entries += 1;
if (buffer->wakeup_cb && !buffer->silent)
buffer->wakeup_cb(buffer->wakeup_cb_ctx);
wakeup = true;
}
pthread_mutex_unlock(&buffer->lock);
if (wakeup)
buffer->wakeup_cb(buffer->wakeup_cb_ctx);
}
}
@ -533,8 +536,9 @@ static void wakeup_log_file(void *p)
{
struct mp_log_root *root = p;
// This makes use of the implicit fact that the caller holds mp_msg_lock.
pthread_mutex_lock(&root->log_file_lock);
pthread_cond_broadcast(&root->log_file_wakeup);
pthread_mutex_unlock(&root->log_file_lock);
}
// Only to be called from the main thread.
@ -658,15 +662,12 @@ void mp_msg_force_stderr(struct mpv_global *global, bool force_stderr)
pthread_mutex_unlock(&mp_msg_lock);
}
// Only to be called from the main thread.
bool mp_msg_has_log_file(struct mpv_global *global)
{
struct mp_log_root *root = global->log->root;
pthread_mutex_lock(&mp_msg_lock);
bool res = !!root->log_file;
pthread_mutex_unlock(&mp_msg_lock);
return res;
return !!root->log_file;
}
void mp_msg_uninit(struct mpv_global *global)