[ 
https://issues.apache.org/jira/browse/QPID-8206?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16505005#comment-16505005
 ] 

Kim van der Riet commented on QPID-8206:
----------------------------------------

I cannot reproduce using this reproducer on my hardware.

*But* if I add a sleep of 10ms into {{InactivityFireEvent::cancel()}} just 
after taking the lock, then the deadlock occurs quite quickly for me - after a 
minute or so of running Pavel's reproducer.
{noformat}
diff --git a/src/qpid/linearstore/JournalImpl.cpp 
b/src/qpid/linearstore/JournalImpl.cpp
index fe919fbd3..e512e9a05 100644
--- a/src/qpid/linearstore/JournalImpl.cpp
+++ b/src/qpid/linearstore/JournalImpl.cpp
@@ -82,6 +82,7 @@ void InactivityFireEvent::fire() {

 void InactivityFireEvent::cancel() {
     ::qpid::sys::Mutex::ScopedLock sl(_ifeStateLock);
+    ::usleep(10 * 1000); // <-- *** DEBUG ***
     ::qpid::sys::TimerTask::cancel();
     _state = CANCELLED;
 }
{noformat}
After making the following fix, but leaving the sleep in place:
{noformat}
diff --git a/src/qpid/linearstore/JournalImpl.cpp 
b/src/qpid/linearstore/JournalImpl.cpp
index fe919fbd3..55f33bb80 100644
--- a/src/qpid/linearstore/JournalImpl.cpp
+++ b/src/qpid/linearstore/JournalImpl.cpp
@@ -81,9 +81,12 @@ void InactivityFireEvent::fire() {
 }

 void InactivityFireEvent::cancel() {
-    ::qpid::sys::Mutex::ScopedLock sl(_ifeStateLock);
     ::qpid::sys::TimerTask::cancel();
-    _state = CANCELLED;
+    {
+        ::qpid::sys::Mutex::ScopedLock sl(_ifeStateLock);
+        ::usleep(10 * 1000); // <-- *** DEBUG ***
+        _state = CANCELLED;
+    }
 }

 GetEventsFireEvent::GetEventsFireEvent(JournalImpl* p,
{noformat}
I cannot get the reproducer to fail. I am hopeful that this will make a valid 
fix. I will continue to test for a while.

In essence, the fix allows the underlying timer to *first* cancel (which 
includes waiting for any flush fires that may have occurred on another thread) 
before taking the lock to change the local state to CANCELLED.

To be sure we are not introducing a regression, we should probably also check 
the reproducers for QPID-7975.

> [linearstore] Deadlock possible in InactivityFireEvent if fire() is called at 
> the same time as cancel()
> -------------------------------------------------------------------------------------------------------
>
>                 Key: QPID-8206
>                 URL: https://issues.apache.org/jira/browse/QPID-8206
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker
>            Reporter: Kim van der Riet
>            Assignee: Kim van der Riet
>            Priority: Major
>
> A deadlock has been observed in InactivityFireEvent if 
> {{InactivityFireEvent::fire()}} triggered by the timer occurs at almost the 
> same time as {{InactivityFireEvent::cancel()}} on another thread, and which 
> occurs if the queue is deleted.
> The mutex {{InactivityFireEvent::_ifeStateLock}} becomes deadlocked if the 
> thread calling {{cancel()}} obtains the lock, but a fire event is imminent. 
> The {{fire()}} call will then be blocked on this mutex. However, cancel 
> cannot complete until fire competes owing to the sys::Time Monitor which 
> waits for all fires to complete before allowing the cancel to occur.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to