changeset 23683:c55138a943c9

Fix issue testing if thin blocks is enabled -- instead of calling the function IsThinBlocksEnabled(), the address of it is tested against 0. Fix locking order issue where cs_mapRelay is taken before cs_vSend, although most of the time the opposite is true. This reverse order triggers the possible deadlock detector. Fix signed/unsigned comparison complaints in leveldb.
author Andrew Stone <g.andrew.stone@gmail.com>
date Mon, 15 Feb 2016 09:08:22 -0500
parents 343b908ab77c
children 5b387556b1d1
files src/leveldb/db/memtable.cc src/leveldb/util/bloom.cc src/leveldb/util/logging.cc src/main.cpp src/sync.cpp
diffstat 5 files changed, 21 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/src/leveldb/db/memtable.cc	Sat Feb 13 20:50:09 2016 -0500
+++ b/src/leveldb/db/memtable.cc	Mon Feb 15 09:08:22 2016 -0500
@@ -101,7 +101,7 @@
   p += 8;
   p = EncodeVarint32(p, val_size);
   memcpy(p, value.data(), val_size);
-  assert((p + val_size) - buf == encoded_len);
+  assert((size_t)((p + val_size) - buf) == encoded_len);
   table_.Insert(buf);
 }
 
--- a/src/leveldb/util/bloom.cc	Sat Feb 13 20:50:09 2016 -0500
+++ b/src/leveldb/util/bloom.cc	Mon Feb 15 09:08:22 2016 -0500
@@ -47,7 +47,7 @@
     dst->resize(init_size + bytes, 0);
     dst->push_back(static_cast<char>(k_));  // Remember # of probes in filter
     char* array = &(*dst)[init_size];
-    for (size_t i = 0; i < n; i++) {
+    for (size_t i = 0; i < (size_t) n; i++) {
       // Use double-hashing to generate a sequence of hash values.
       // See analysis in [Kirsch,Mitzenmacher 2006].
       uint32_t h = BloomHash(keys[i]);
--- a/src/leveldb/util/logging.cc	Sat Feb 13 20:50:09 2016 -0500
+++ b/src/leveldb/util/logging.cc	Mon Feb 15 09:08:22 2016 -0500
@@ -52,7 +52,7 @@
     char c = (*in)[0];
     if (c >= '0' && c <= '9') {
       ++digits;
-      const int delta = (c - '0');
+      const unsigned int delta = (c - '0');
       static const uint64_t kMaxUint64 = ~static_cast<uint64_t>(0);
       if (v > kMaxUint64/10 ||
           (v == kMaxUint64/10 && delta > kMaxUint64%10)) {
--- a/src/main.cpp	Sat Feb 13 20:50:09 2016 -0500
+++ b/src/main.cpp	Mon Feb 15 09:08:22 2016 -0500
@@ -4235,11 +4235,18 @@
                 // Send stream from relay memory
                 bool pushed = false;
                 {
-                    LOCK(cs_mapRelay);
-                    map<CInv, CDataStream>::iterator mi = mapRelay.find(inv);
-                    if (mi != mapRelay.end()) {
-                        pfrom->PushMessage(inv.GetCommand(), (*mi).second);
+                  CDataStream cd(0,0);
+                  if (1)
+                    {
+                      LOCK(cs_mapRelay);  // BU: We need to release this lock before push message or there is a potential deadlock because cs_vSend is often taken before cs_mapRelay
+                      map<CInv, CDataStream>::iterator mi = mapRelay.find(inv);
+                      if (mi != mapRelay.end()) {
+                        cd = (*mi).second; // I have to copy, because .second may be deleted once lock is released
                         pushed = true;
+                      }
+                    }
+                    if (pushed) {
+                        pfrom->PushMessage(inv.GetCommand(), cd);                       
                     }
                 }
                 if (!pushed && inv.type == MSG_TX) {
@@ -4459,7 +4466,7 @@
             // nodes)
 
             // BUIP010 Extreme Thinblocks: We only do inv/getdata for xthinblocks and so we must have headersfirst turned off
-            if (!IsThinBlocksEnabled)
+            if (!IsThinBlocksEnabled())
                 pfrom->PushMessage(NetMsgType::SENDHEADERS);
         }
     }
@@ -4535,7 +4542,7 @@
     {
         LOCK(cs_main);
         // BUIP010 Xtreme Thinblocks: We only do inv/getdata for xthinblocks and so we must have headersfirst turned off
-        if (IsThinBlocksEnabled)
+        if (IsThinBlocksEnabled())
             State(pfrom->GetId())->fPreferHeaders = false;
         else
             State(pfrom->GetId())->fPreferHeaders = true;
@@ -4976,7 +4983,7 @@
                         pindexLast->GetBlockHash().ToString(),
                         pindexLast->nHeight);
             //} else {   BU: We don't support headers first for XThinblocks.
-            } else if (!IsThinBlocksEnabled) {
+            } else if (!IsThinBlocksEnabled()) {
                 vector<CInv> vGetData;
                 // Download as much as possible, from earliest to latest.
                 BOOST_REVERSE_FOREACH(CBlockIndex *pindex, vToFetch) {
--- a/src/sync.cpp	Sat Feb 13 20:50:09 2016 -0500
+++ b/src/sync.cpp	Mon Feb 15 09:08:22 2016 -0500
@@ -120,20 +120,20 @@
     dd_mutex.lock();
 
     (*lockstack).push_back(std::make_pair(c, locklocation));
-
+    // If this is a blocking lock operation, we want to make sure that the locking order between 2 mutexes is consistent across the program
     if (!fTry) {
         BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, (*lockstack)) {
             if (i.first == c)
                 break;
 
             std::pair<void*, void*> p1 = std::make_pair(i.first, c);
-            if (lockorders.count(p1))
+            if (lockorders.count(p1))  // If this order has already been placed into the order map, we've already tested it
                 continue;
             lockorders[p1] = (*lockstack);
-
+            // check to see if the opposite order has ever occurred, if so flag a possible deadlock
             std::pair<void*, void*> p2 = std::make_pair(c, i.first);
             if (lockorders.count(p2))
-                potential_deadlock_detected(p1, lockorders[p2], lockorders[p1]);
+                potential_deadlock_detected(p1, lockorders[p1], lockorders[p2]);
         }
     }
     dd_mutex.unlock();