[
https://issues.apache.org/jira/browse/DISPATCH-1956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17358642#comment-17358642
]
ASF GitHub Bot commented on DISPATCH-1956:
------------------------------------------
kgiusti commented on a change in pull request #1243:
URL: https://github.com/apache/qpid-dispatch/pull/1243#discussion_r646634535
##########
File path: src/log.c
##########
@@ -592,24 +601,22 @@ qd_error_t qd_log_entity(qd_entity_t *entity)
QD_ERROR_BREAK();
}
- //
- // Obtain the log_source_lock lock. This lock is also used when
write_log() is called.
- //
- sys_mutex_lock(log_source_lock);
-
- qd_log_source_t *src = qd_log_source_lh(module); /* The
original(already existing) log source */
+ qd_log_source_t *src = qd_log_source_find_or_make(module, false); /*
The original (already existing) log source */
if (has_output_file) {
+ sys_mutex_lock(log_source_lock);
log_sink_t* sink = log_sink_lh(outputFile);
+ sys_mutex_unlock(log_source_lock);
if (!sink) {
error_in_output = true;
- sys_mutex_unlock(log_source_lock);
break;
}
// DEFAULT source may already have a sink, so free the old sink
first
if (src->sink) {
+ sys_mutex_lock(log_source_lock);
log_sink_free_lh(src->sink);
+ sys_mutex_unlock(log_source_lock);
Review comment:
This could be a problem: src->sink has been freed. Once the lock is
dropped some other thread may try to access that stale sink pointer.
##########
File path: src/log.c
##########
@@ -592,24 +601,22 @@ qd_error_t qd_log_entity(qd_entity_t *entity)
QD_ERROR_BREAK();
}
- //
- // Obtain the log_source_lock lock. This lock is also used when
write_log() is called.
- //
- sys_mutex_lock(log_source_lock);
-
- qd_log_source_t *src = qd_log_source_lh(module); /* The
original(already existing) log source */
+ qd_log_source_t *src = qd_log_source_find_or_make(module, false); /*
The original (already existing) log source */
if (has_output_file) {
+ sys_mutex_lock(log_source_lock);
log_sink_t* sink = log_sink_lh(outputFile);
+ sys_mutex_unlock(log_source_lock);
Review comment:
What prevents another thread from calling this function and freeing the
same sink (line 618) once the first thread drops the lock?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Potential deadlock: logging lock vs entity cache lock
> -----------------------------------------------------
>
> Key: DISPATCH-1956
> URL: https://issues.apache.org/jira/browse/DISPATCH-1956
> Project: Qpid Dispatch
> Issue Type: Bug
> Components: Router Node
> Affects Versions: 1.15.0
> Reporter: Ken Giusti
> Assignee: Michael Goulish
> Priority: Major
> Labels: deadlock, tsan
> Fix For: 1.17.0
>
> Attachments: tsan.supp
>
>
> {noformat}
> WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock)
> (pid=1474955)
> Cycle in lock order graph: M11 (0x7b10000002c0) => M9 (0x7b1000000240) =>
> M11
>
> Mutex M9 acquired here while holding mutex M11 in main thread:
> #0 pthread_mutex_lock <null> (libtsan.so.0+0x528ac)
> #1 sys_mutex_lock
> /home/kgiusti/work/dispatch/qpid-dispatch/src/posix/threading.c:57
> (libqpid-dispatch.so+0x8cb7d)
> #2 push_event
> /home/kgiusti/work/dispatch/qpid-dispatch/src/entity_cache.c:63
> (libqpid-dispatch.so+0x6fa13)
> #3 qd_entity_cache_add
> /home/kgiusti/work/dispatch/qpid-dispatch/src/entity_cache.c:69
> (libqpid-dispatch.so+0x6fc26)
> #4 qd_alloc_init
> /home/kgiusti/work/dispatch/qpid-dispatch/src/alloc_pool.c:302
> (libqpid-dispatch.so+0x5878b)
> #5 qd_alloc /home/kgiusti/work/dispatch/qpid-dispatch/src/alloc_pool.c:318
> (libqpid-dispatch.so+0x5878b)
> #6 new_qd_log_entry_t /home/kgiusti/work/dispatch/qpid-dispatch/src/log.c:61
> (libqpid-dispatch.so+0x75891)
> #7 qd_vlog_impl /home/kgiusti/work/dispatch/qpid-dispatch/src/log.c:426
> (libqpid-dispatch.so+0x76205)
> #8 qd_log_impl /home/kgiusti/work/dispatch/qpid-dispatch/src/log.c:453
> (libqpid-dispatch.so+0x76580)
> #9 qd_python_log
> /home/kgiusti/work/dispatch/qpid-dispatch/src/python_embedded.c:547
> (libqpid-dispatch.so+0x8d1cb)
> #10 <null> <null> (libpython3.8.so.1.0+0x12a23b)
> #11 main_process
> /home/kgiusti/work/dispatch/qpid-dispatch/router/src/main.c:95
> (qdrouterd+0x40281c)
> #12 main /home/kgiusti/work/dispatch/qpid-dispatch/router/src/main.c:367
> (qdrouterd+0x4024fc)
>
> Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative
> warning message
>
> Mutex M11 acquired here while holding mutex M9 in main thread:
> #0 pthread_mutex_lock <null> (libtsan.so.0+0x528ac)
> #1 sys_mutex_lock
> /home/kgiusti/work/dispatch/qpid-dispatch/src/posix/threading.c:57
> (libqpid-dispatch.so+0x8cb7d)
> #2 qd_vlog_impl /home/kgiusti/work/dispatch/qpid-dispatch/src/log.c:425
> (libqpid-dispatch.so+0x76200)
> #3 qd_log_impl /home/kgiusti/work/dispatch/qpid-dispatch/src/log.c:453
> (libqpid-dispatch.so+0x76580)
> #4 qd_python_log
> /home/kgiusti/work/dispatch/qpid-dispatch/src/python_embedded.c:547
> (libqpid-dispatch.so+0x8d1cb)
> #5 <null> <null> (libpython3.8.so.1.0+0x12a23b)
> #6 main_process
> /home/kgiusti/work/dispatch/qpid-dispatch/router/src/main.c:95
> (qdrouterd+0x40281c)
> #7 main /home/kgiusti/work/dispatch/qpid-dispatch/router/src/main.c:367
> (qdrouterd+0x4024fc)
>
> SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock)
> (/lib64/libtsan.so.0+0x528ac) in __interceptor_pthread_mutex_lock
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
