Remove race condition in low level IO when throttling input

to avoid overloading the clustering service


git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@752560 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 7 insertions, 5 deletions

diff --git a/cpp/src/qpid/sys/AsynchIOHandler.cpp b/cpp/src/qpid/sys/AsynchIOHandler.cpp
index 3bc05e4bf9..6b7e7b5145 100644
--- a/cpp/src/qpid/sys/AsynchIOHandler.cpp
+++ b/cpp/src/qpid/sys/AsynchIOHandler.cpp
@@ -84,10 +84,11 @@ void AsynchIOHandler::giveReadCredit(int32_t credit) {
     // Check whether we started in the don't about credit state
     if (readCredit.boolCompareAndSwap(InfiniteCredit, credit))
         return;
-    else if (readCredit.fetchAndAdd(credit) != 0)
-        return;
-    // Lock and retest credit to make sure we don't race with decreasing credit
+    // TODO In theory should be able to use an atomic operation before taking the lock
+    // but in practice there seems to be an unexplained race in that case
     ScopedLock<Mutex> l(creditLock);
+    if (readCredit.fetchAndAdd(credit) != 0)
+        return;
     assert(readCredit.get() >= 0);
     if (readCredit.get() != 0)
         aio->startReading();
@@ -141,9 +142,10 @@ bool AsynchIOHandler::readbuff(AsynchIO& , AsynchIO::BufferBase* buff) {
     }
     // Check here for read credit
     if (readCredit.get() != InfiniteCredit) {
+        // TODO In theory should be able to use an atomic operation before taking the lock
+        // but in practice there seems to be an unexplained race in that case
+        ScopedLock<Mutex> l(creditLock);
         if (--readCredit == 0) {
-            // Lock and retest credit to make sure we don't race with increasing credit
-            ScopedLock<Mutex> l(creditLock);
             assert(readCredit.get() >= 0);
             if (readCredit.get() == 0) {
                 aio->stopReading();