Merge pull request #990 from evoskuil/master

Fix thread safety in protocol_block_in_31800, update logging.

Author: evoskuil

include/bitcoin/node/protocols/protocol_block_in_31800.hpp

@@ -67,6 +67,7 @@ class BCN_API protocol_block_in_31800
     bool is_idle() const NOEXCEPT override;
     virtual void do_purge(peer_t) NOEXCEPT;
     virtual void do_split(peer_t) NOEXCEPT;
+    virtual void do_stall(peer_t) NOEXCEPT;
     virtual void do_report(count_t count) NOEXCEPT;
 
     /// Check incoming block message.

src/chasers/chaser_confirm.cpp

@@ -350,7 +350,7 @@ bool chaser_confirm::complete_block(const code& ec, const header_link& link,
     // CONFIRMABLE BLOCK
     notify(error::success, chase::confirmable, link);
     fire(events::block_confirmed, height);
-    LOGV("Block confirmable: " << height << (bypass ? " (bypass)" : ""));
+    LOGV("Block confirmed: " << height << (bypass ? " (bypass)" : ""));
     return true;
 }
 

src/protocols/protocol_block_in_31800.cpp

@@ -92,6 +92,7 @@ void protocol_block_in_31800::stopping(const code& ec) NOEXCEPT
 
 bool protocol_block_in_31800::is_idle() const NOEXCEPT
 {
+    BC_ASSERT(stranded());
     return map_->empty();
 }
 
@@ -116,22 +117,14 @@ bool protocol_block_in_31800::handle_event(const code&, chase event_,
             // If this channel has divisible work, split it and stop.
             // There are no channels reporting work, either stalled or done.
             // This is initiated by any channel notifying chase::starved.
-            if (map_->size() > one)
-            {
-                POST(do_split, peer_t{});
-            }
-
+            POST(do_stall, peer_t{});
             break;
         }
         case chase::purge:
         {
             // If have work clear it and stop.
             // This is initiated by chase::regressed/disorganized.
-            if (map_->size() > one)
-            {
-                POST(do_purge, peer_t{});
-            }
-
+            POST(do_purge, peer_t{});
             break;
         }
         case chase::download:
@@ -162,38 +155,44 @@ bool protocol_block_in_31800::handle_event(const code&, chase event_,
     return true;
 }
 
+void protocol_block_in_31800::do_report(count_t sequence) NOEXCEPT
+{
+    BC_ASSERT(stranded());
+
+    // Uses application logging since it outputs to a runtime option.
+    LOGA("Work report [" << sequence << "] is (" << map_->size() << ") for ["
+        << opposite() << "].");
+}
+
 void protocol_block_in_31800::do_get_downloads(count_t) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
-    if (stopped())
+    if (stopped() || !is_idle())
         return;
 
-    if (is_idle())
-    {
-        // Assume performance was stopped due to exhaustion.
-        start_performance();
-        get_hashes(BIND(handle_get_hashes, _1, _2, _3));
-    }
+    // Assume performance was stopped due to exhaustion.
+    start_performance();
+    get_hashes(BIND(handle_get_hashes, _1, _2, _3));
 }
 
 void protocol_block_in_31800::do_purge(peer_t) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
-    if (!map_->empty())
-    {
-        LOGV("Purge work (" << map_->size() << ") from [" << opposite() << "].");
-        map_->clear();
-        stop(error::sacrificed_channel);
-    }
+    if (map_->empty())
+        return;
+
+    LOGV("Purge work (" << map_->size() << ") from [" << opposite() << "].");
+    map_->clear();
+    stop(error::sacrificed_channel);
 }
 
-void protocol_block_in_31800::do_split(peer_t) NOEXCEPT
+void protocol_block_in_31800::do_stall(peer_t) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
-    if (stopped())
+    if (stopped() || (map_->size() <= one))
         return;
 
     LOGV("Divide work (" << map_->size() << ") from [" << opposite() << "].");
@@ -203,13 +202,18 @@ void protocol_block_in_31800::do_split(peer_t) NOEXCEPT
     stop(error::sacrificed_channel);
 }
 
-void protocol_block_in_31800::do_report(count_t sequence) NOEXCEPT
+void protocol_block_in_31800::do_split(peer_t) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
-    // Uses application logging since it outputs to a runtime option.
-    LOGA("Work report [" << sequence << "] is (" << map_->size() << ") for ["
-        << opposite() << "].");
+    if (stopped() || (map_->size() <= one))
+        return;
+
+    LOGV("Split work (" << map_->size() << ") from [" << opposite() << "].");
+    restore(chaser_check::split(map_));
+    restore(map_);
+    map_ = chaser_check::empty_map();
+    stop(error::sacrificed_channel);
 }
 
 // request hashes

src/protocols/protocol_header_out_70012.cpp

@@ -112,7 +112,8 @@ bool protocol_header_out_70012::do_announce(header_t link) NOEXCEPT
         return true;
     }
 
-    LOGN("Announce " << encode_hash(hash) << " to [" << opposite() << "].");
+    LOGN("Announce ..." << encode_hash(hash).substr(hash_size - 8, 8)
+        << " to [" << opposite() << "].");
     SEND(headers{ { ptr } }, handle_send, _1);
     return true;
 }