miner: Avoid stack-use-after-return in validationinterface #18742

pull MarcoFalke wants to merge 4 commits into bitcoin:master from MarcoFalke:2004-minerNoCrash changing 5 files +75 −34
  1. MarcoFalke commented at 7:10 PM on April 22, 2020: member

    When a validationinterface has itself unregistered in one thread, but is about to get executed in another thread [1], there is a race:

    • The validationinterface destructing itself
    • The validationinterface getting dereferenced for execution

    [1] https://github.com/bitcoin/bitcoin/blob/64139803f1225dab26197a20314109d37fa87d5f/src/validationinterface.cpp#L82-L83

    This happens in the miner. More generally it happens everywhere where at least one thread is generating notifications and another one is unregistering a validationinterface.

    This issue has been fixed in commit ab31b9d6fe7b39713682e3f52d11238dbe042c16, but the fix has not been applied to the miner.

    Example where this happened in practice: https://travis-ci.org/github/bitcoin/bitcoin/jobs/675322230#L4414

  2. MarcoFalke added the label Mining on Apr 22, 2020
  3. MarcoFalke added the label Needs backport (0.19) on Apr 22, 2020
  4. MarcoFalke added the label Needs backport (0.18) on Apr 22, 2020
  5. MarcoFalke added the label Needs backport (0.20) on Apr 22, 2020
  6. MarcoFalke added this to the milestone 0.20.0 on Apr 22, 2020
  7. DrahtBot commented at 7:34 PM on April 22, 2020: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18531 (rpc: Assert that RPCArg names are equal to CRPCCommand ones by MarcoFalke)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  8. practicalswift commented at 7:54 PM on April 22, 2020: contributor

    Oh, great find!

    How did you find this one and in what commit was it introduced? :)

  9. MarcoFalke commented at 8:39 PM on April 22, 2020: member

    <details><summary>Good question. This is the traceback:</summary>

    118/145 - rpc_getblockstats.py failed, Duration: 3 s
    
    stdout:
    
    2020-04-15T15:10:09.960000Z TestFramework (INFO): Initializing test directory /home/travis/build/bitcoin/bitcoin/ci/scratch/test_runner/test_runner_₿_🏃_20200415_145914/rpc_getblockstats_25
    
    2020-04-15T15:10:12.198000Z TestFramework (ERROR): Unexpected exception caught during testing
    
    Traceback (most recent call last):
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 108, in _request
    
        return self._get_response()
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 166, in _get_response
    
        http_response = self.__conn.getresponse()
    
      File "/usr/lib/python3.6/http/client.py", line 1346, in getresponse
    
        response.begin()
    
      File "/usr/lib/python3.6/http/client.py", line 307, in begin
    
        version, status, reason = self._read_status()
    
      File "/usr/lib/python3.6/http/client.py", line 276, in _read_status
    
        raise RemoteDisconnected("Remote end closed connection without"
    
    http.client.RemoteDisconnected: Remote end closed connection without response
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 112, in main
    
        self.run_test()
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_getblockstats.py", line 97, in run_test
    
        self.load_test_data(test_data)
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_getblockstats.py", line 89, in load_test_data
    
        self.nodes[0].submitblock(b)
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
    
        return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 142, in __call__
    
        response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 113, in _request
    
        self.__conn.request(method, path, postdata, headers)
    
      File "/usr/lib/python3.6/http/client.py", line 1254, in request
    
        self._send_request(method, url, body, headers, encode_chunked)
    
      File "/usr/lib/python3.6/http/client.py", line 1300, in _send_request
    
        self.endheaders(body, encode_chunked=encode_chunked)
    
      File "/usr/lib/python3.6/http/client.py", line 1249, in endheaders
    
        self._send_output(message_body, encode_chunked=encode_chunked)
    
      File "/usr/lib/python3.6/http/client.py", line 1036, in _send_output
    
        self.send(msg)
    
      File "/usr/lib/python3.6/http/client.py", line 974, in send
    
        self.connect()
    
      File "/usr/lib/python3.6/http/client.py", line 946, in connect
    
        (self.host,self.port), self.timeout, self.source_address)
    
      File "/usr/lib/python3.6/socket.py", line 724, in create_connection
    
        raise err
    
      File "/usr/lib/python3.6/socket.py", line 713, in create_connection
    
        sock.connect(sa)
    
    ConnectionRefusedError: [Errno 111] Connection refused
    
    2020-04-15T15:10:12.261000Z TestFramework (INFO): Stopping nodes
    
    2020-04-15T15:10:12.261000Z TestFramework.node0 (ERROR): Unable to stop node.
    
    Traceback (most recent call last):
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_node.py", line 266, in stop_node
    
        self.stop(wait=wait)
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
    
        return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 142, in __call__
    
        response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 107, in _request
    
        self.__conn.request(method, path, postdata, headers)
    
      File "/usr/lib/python3.6/http/client.py", line 1254, in request
    
        self._send_request(method, url, body, headers, encode_chunked)
    
      File "/usr/lib/python3.6/http/client.py", line 1265, in _send_request
    
        self.putrequest(method, url, **skips)
    
      File "/usr/lib/python3.6/http/client.py", line 1118, in putrequest
    
        raise CannotSendRequest(self.__state)
    
    http.client.CannotSendRequest: Request-sent
    
    [node 0] Cleaning up leftover process
    
    stderr:
    
    Traceback (most recent call last):
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_getblockstats.py", line 165, in <module>
    
        GetblockstatsTest().main()
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 135, in main
    
        exit_code = self.shutdown()
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 242, in shutdown
    
        self.stop_nodes()
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 453, in stop_nodes
    
        node.stop_node(wait=wait)
    
      File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_node.py", line 280, in stop_node
    
        raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
    
    AssertionError: Unexpected stderr =================================================================
    
    ==3550==ERROR: AddressSanitizer: stack-use-after-return on address 0x7fdbfbb7dc40 at pc 0x5567a688f34f bp 0x7fdbfee66f10 sp 0x7fdbfee66f08
    
    READ of size 8 at 0x7fdbfbb7dc40 thread T4 (b-httpworker.0)
    
        [#0](/bitcoin-bitcoin/0/) 0x5567a688f34e in CMainSignals::NewPoWValidBlock(CBlockIndex const*, std::shared_ptr<CBlock const> const&)::$_15::operator()(CValidationInterface&) const /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/validationinterface.cpp:246:75
    
        [#1](/bitcoin-bitcoin/1/) 0x5567a688f34e in void MainSignalsInstance::Iterate<CMainSignals::NewPoWValidBlock(CBlockIndex const*, std::shared_ptr<CBlock const> const&)::$_15>(CMainSignals::NewPoWValidBlock(CBlockIndex const*, std::shared_ptr<CBlock const> const&)::$_15&&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/validationinterface.cpp:83
    
        [#2](/bitcoin-bitcoin/2/) 0x5567a688f34e in CMainSignals::NewPoWValidBlock(CBlockIndex const*, std::shared_ptr<CBlock const> const&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/validationinterface.cpp:246
    
        [#3](/bitcoin-bitcoin/3/) 0x5567a67a6cf1 in CChainState::AcceptBlock(std::shared_ptr<CBlock const> const&, BlockValidationState&, CChainParams const&, CBlockIndex**, bool, FlatFilePos const*, bool*) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/validation.cpp:3758:26
    
        [#4](/bitcoin-bitcoin/4/) 0x5567a67aa38c in ProcessNewBlock(CChainParams const&, std::shared_ptr<CBlock const>, bool, bool*) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/validation.cpp:3798:40
    
        [#5](/bitcoin-bitcoin/5/) 0x5567a64467b6 in submitblock(JSONRPCRequest const&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/mining.cpp:947:21
    
        [#6](/bitcoin-bitcoin/6/) 0x5567a63fe3e6 in CRPCCommand::CRPCCommand(char const*, char const*, UniValue (*)(JSONRPCRequest const&), std::initializer_list<char const*>)::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/./rpc/server.h:104:94
    
        [#7](/bitcoin-bitcoin/7/) 0x5567a63fdce8 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(char const*, char const*, UniValue (*)(JSONRPCRequest const&), std::initializer_list<char const*>)::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/std_function.h:301:9
    
        [#8](/bitcoin-bitcoin/8/) 0x5567a602f02e in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/std_function.h:706:14
    
        [#9](/bitcoin-bitcoin/9/) 0x5567a654f18f in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/server.cpp:452:20
    
        [#10](/bitcoin-bitcoin/10/) 0x5567a654ea20 in CRPCTable::execute(JSONRPCRequest const&) const /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/server.cpp:435:17
    
        [#11](/bitcoin-bitcoin/11/) 0x5567a6942a88 in HTTPReq_JSONRPC(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/httprpc.cpp:208:40
    
        [#12](/bitcoin-bitcoin/12/) 0x5567a6386af0 in std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), bool (*)(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/std_function.h:301:9
    
        [#13](/bitcoin-bitcoin/13/) 0x5567a69631e9 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/std_function.h:706:14
    
        [#14](/bitcoin-bitcoin/14/) 0x5567a6962e2b in HTTPWorkItem::operator()() /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/httpserver.cpp:55:9
    
        [#15](/bitcoin-bitcoin/15/) 0x5567a696c777 in WorkQueue<HTTPClosure>::Run() /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/httpserver.cpp:114:13
    
        [#16](/bitcoin-bitcoin/16/) 0x5567a695507f in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/httpserver.cpp:342:12
    
        [#17](/bitcoin-bitcoin/17/) 0x5567a6972c29 in void std::__invoke_impl<void, void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>(std::__invoke_other, void (*&&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&&, int&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/invoke.h:60:14
    
        [#18](/bitcoin-bitcoin/18/) 0x5567a6972aba in std::__invoke_result<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>::type std::__invoke<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>(void (*&&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&&, int&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/invoke.h:95:14
    
        [#19](/bitcoin-bitcoin/19/) 0x5567a6972881 in decltype(std::__invoke(_S_declval<0ul>(), _S_declval<1ul>(), _S_declval<2ul>())) std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> >::_M_invoke<0ul, 1ul, 2ul>(std::_Index_tuple<0ul, 1ul, 2ul>) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/thread:234:13
    
        [#20](/bitcoin-bitcoin/20/) 0x5567a6971fe1 in std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> >::operator()() /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/thread:243:11
    
        [#21](/bitcoin-bitcoin/21/) 0x5567a6971fe1 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> > >::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/thread:186
    
        [#22](/bitcoin-bitcoin/22/) 0x7fdc0a3a76de  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbd6de)
    
        [#23](/bitcoin-bitcoin/23/) 0x7fdc0b3436da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    
        [#24](/bitcoin-bitcoin/24/) 0x7fdc0965888e in clone (/lib/x86_64-linux-gnu/libc.so.6+0x12188e)
    
    Address 0x7fdbfbb7dc40 is located in stack of thread T6 (b-httpworker.2) at offset 3136 in frame
    
        [#0](/bitcoin-bitcoin/0/) 0x5567a6445a2f in submitblock(JSONRPCRequest const&) /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/mining.cpp:896
    
      This frame has 57 object(s):
    
        [32, 33) 'ref.tmp.i'
    
        [48, 192) 'ref.tmp' (line 898)
    
        [256, 288) 'agg.tmp'
    
        [320, 321) 'ref.tmp1' (line 898)
    
        [336, 368) 'agg.tmp6'
    
        [400, 401) 'ref.tmp7' (line 898)
    
        [416, 440) 'agg.tmp16'
    
        [480, 864) 'ref.tmp18' (line 898)
    
        [928, 960) 'agg.tmp19'
    
        [992, 993) 'ref.tmp20' (line 898)
    
        [1008, 1048) 'agg.tmp29'
    
        [1088, 1092) 'ref.tmp30' (line 898)
    
        [1104, 1136) 'agg.tmp37'
    
        [1168, 1169) 'ref.tmp38' (line 898)
    
        [1184, 1216) 'agg.tmp47'
    
        [1248, 1249) 'ref.tmp48' (line 898)
    
        [1264, 1288) 'agg.tmp57'
    
        [1328, 1360) 'agg.tmp64'
    
        [1392, 1393) 'ref.tmp65' (line 898)
    
        [1408, 1448) 'agg.tmp74'
    
        [1488, 1520) 'agg.tmp81'
    
        [1552, 1553) 'ref.tmp82' (line 898)
    
        [1568, 1600) 'agg.tmp91'
    
        [1632, 1633) 'ref.tmp92' (line 898)
    
        [1648, 1672) 'agg.tmp101'
    
        [1712, 1713) 'ref.tmp108' (line 898)
    
        [1728, 1752) 'agg.tmp117'
    
        [1792, 1928) 'agg.tmp118'
    
        [2000, 2032) 'agg.tmp119'
    
        [2064, 2065) 'ref.tmp120' (line 898)
    
        [2080, 2112) 'agg.tmp129'
    
        [2144, 2145) 'ref.tmp130' (line 898)
    
        [2160, 2184) 'agg.tmp139'
    
        [2224, 2256) 'agg.tmp150'
    
        [2288, 2320) 'agg.tmp151'
    
        [2352, 2384) 'ref.tmp152' (line 898)
    
        [2416, 2448) 'ref.tmp153' (line 898)
    
        [2480, 2481) 'ref.tmp154' (line 898)
    
        [2496, 2528) 'ref.tmp165' (line 898)
    
        [2560, 2561) 'ref.tmp166' (line 898)
    
        [2576, 2608) 'ref.tmp181' (line 898)
    
        [2640, 2672) 'ref.tmp182' (line 898)
    
        [2704, 2705) 'ref.tmp183' (line 898)
    
        [2720, 2752) 'ref.tmp194' (line 898)
    
        [2784, 2785) 'ref.tmp195' (line 898)
    
        [2800, 2816) 'blockptr' (line 912)
    
        [2832, 2864) 'ref.tmp326' (line 915)
    
        [2896, 2897) 'ref.tmp327' (line 915)
    
        [2912, 2944) 'ref.tmp369' (line 919)
    
        [2976, 2977) 'ref.tmp370' (line 919)
    
        [2992, 3024) 'hash' (line 922)
    
        [3056, 3072) 'criticalblock37' (line 924)
    
        [3088, 3104) 'criticalblock38' (line 937)
    
        [3120, 3121) 'new_block' (line 944)
    
        [3136, 3256) 'sc' (line 945) <== Memory access at offset 3136 is inside this variable
    
        [3296, 3328) 'ref.tmp469' (line 945)
    
        [3360, 3376) 'agg.tmp491'
    
    HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
    
          (longjmp and C++ exceptions *are* supported)
    
    Thread T6 (b-httpworker.2) created by T0 here:
    
        [#0](/bitcoin-bitcoin/0/) 0x5567a5fbe02d in pthread_create (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoind+0x1bc202d)
    
        [#1](/bitcoin-bitcoin/1/) 0x7fdc0a3a7994 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbd994)
    
        [#2](/bitcoin-bitcoin/2/) 0x5567a69717a0 in void __gnu_cxx::new_allocator<std::thread>::construct<std::thread, void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&>(std::thread*, void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/ext/new_allocator.h:136:23
    
        [#3](/bitcoin-bitcoin/3/) 0x5567a695c31b in void std::vector<std::thread, std::allocator<std::thread> >::emplace_back<void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&>(void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/vector.tcc:105:4
    
    SUMMARY: AddressSanitizer: stack-use-after-return /home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/validationinterface.cpp:246:75 in CMainSignals::NewPoWValidBlock(CBlockIndex const*, std::shared_ptr<CBlock const> const&)::$_15::operator()(CValidationInterface&) const
    
    Shadow bytes around the buggy address:
    
      0x0ffbff767b30: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767b40: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767b50: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767b60: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767b70: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
    =>0x0ffbff767b80: f5 f5 f5 f5 f5 f5 f5 f5[f5]f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767b90: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767ba0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767bb0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767bc0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
      0x0ffbff767bd0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
    
    Shadow byte legend (one shadow byte represents 8 application bytes):
    
      Addressable:           00
    
      Partially addressable: 01 02 03 04 05 06 07 
    
      Heap left redzone:       fa
    
      Freed heap region:       fd
    
      Stack left redzone:      f1
    
      Stack mid redzone:       f2
    
      Stack right redzone:     f3
    
      Stack after return:      f5
    
      Stack use after scope:   f8
    
      Global redzone:          f9
    
      Global init order:       f6
    
      Poisoned by user:        f7
    
      Container overflow:      fc
    
      Array cookie:            ac
    
      Intra object redzone:    bb
    
      ASan internal:           fe
    
      Left alloca redzone:     ca
    
      Right alloca redzone:    cb
    
      Shadow gap:              cc
    
    Thread T4 (b-httpworker.0) created by T0 here:
    
        [#0](/bitcoin-bitcoin/0/) 0x5567a5fbe02d in pthread_create (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoind+0x1bc202d)
    
        [#1](/bitcoin-bitcoin/1/) 0x7fdc0a3a7994 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbd994)
    
        [#2](/bitcoin-bitcoin/2/) 0x5567a69717a0 in void __gnu_cxx::new_allocator<std::thread>::construct<std::thread, void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&>(std::thread*, void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/ext/new_allocator.h:136:23
    
        [#3](/bitcoin-bitcoin/3/) 0x5567a695c31b in void std::vector<std::thread, std::allocator<std::thread> >::emplace_back<void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&>(void (&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&, int&) /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/vector.tcc:105:4
    
    ==3550==ABORTING != 
    

    </details>

  10. MarcoFalke removed the label Needs backport (0.18) on Apr 22, 2020
  11. MarcoFalke removed the label Needs backport (0.19) on Apr 22, 2020
  12. MarcoFalke commented at 8:54 PM on April 22, 2020: member

    I might write a test tomorrow with steps to reproduce

  13. MarcoFalke force-pushed on Apr 22, 2020
  14. MarcoFalke force-pushed on Apr 22, 2020
  15. MarcoFalke force-pushed on Apr 23, 2020
  16. MarcoFalke commented at 6:24 PM on April 23, 2020: member

    @practicalswift This is a race, so I couldn't find a test that is reproducible. Though, I have attached a unit test, that fails when run long enough in a loop.

    All you need to do is ./configure --with-sanitizers=address and then run the tests with

    export ASAN_OPTIONS=detect_leaks=0
    make
    while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no  ; do true; done
    
  17. MarcoFalke commented at 8:47 PM on April 23, 2020: member

    If someone has issues reproducing the crash in the unit test, the following diff might help:

    diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
    index 11000774c0..6a311eeb44 100644
    --- a/src/validationinterface.cpp
    +++ b/src/validationinterface.cpp
    @@ -11,6 +11,7 @@
     #include <primitives/block.h>
     #include <primitives/transaction.h>
     #include <scheduler.h>
    +#include <util/time.h>
     
     #include <future>
     #include <unordered_map>
    @@ -80,6 +81,7 @@ public:
                 ++it->count;
                 {
                     REVERSE_LOCK(lock);
    +                UninterruptibleSleep(std::chrono::microseconds(500));
                     f(*it->callbacks);
                 }
                 it = --it->count ? std::next(it) : m_list.erase(it);
    
  18. fanquake commented at 5:36 AM on April 26, 2020: member

    Concept ACK. Running just fa9f3a51897d5457492f84f7c8a41fea783d687d (no additional patch), I generally see a crash between 5 - 20 iterations:

    /bitcoin# while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no  ; do true; done
    Running 1 test case...
    
    *** No errors detected
    Running 1 test case...
    
    *** No errors detected
    Running 1 test case...
    
    *** No errors detected
    Running 1 test case...
    
    *** No errors detected
    Running 1 test case...
    =================================================================
    ==95792==ERROR: AddressSanitizer: unknown-crash on address 0x7f8f04f60dc0 at pc 0x55abc9388623 bp 0x7f8f057777f0 sp 0x7f8f057777e8
    READ of size 8 at 0x7f8f04f60dc0 thread T4
        [#0](/bitcoin-bitcoin/0/) 0x55abc9388622 in CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&)::$_14::operator()(CValidationInterface&) const /bitcoin/src/validationinterface.cpp:241:75
        [#1](/bitcoin-bitcoin/1/) 0x55abc9388622 in void MainSignalsInstance::Iterate<CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&)::$_14>(CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&)::$_14&&) /bitcoin/src/validationinterface.cpp:83
        [#2](/bitcoin-bitcoin/2/) 0x55abc9388622 in CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&) /bitcoin/src/validationinterface.cpp:241
        [#3](/bitcoin-bitcoin/3/) 0x55abc8b9c94c in validationinterface_tests::unregister_validation_interface_race::test_method()::$_0::operator()() const /bitcoin/src/test/validationinterface_tests.cpp:28:30
        [#4](/bitcoin-bitcoin/4/) 0x55abc8b9c94c in void std::__invoke_impl<void, validationinterface_tests::unregister_validation_interface_race::test_method()::$_0>(std::__invoke_other, validationinterface_tests::unregister_validation_interface_race::test_method()::$_0&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:60
        [#5](/bitcoin-bitcoin/5/) 0x55abc8b9c94c in std::__invoke_result<validationinterface_tests::unregister_validation_interface_race::test_method()::$_0>::type std::__invoke<validationinterface_tests::unregister_validation_interface_race::test_method()::$_0>(validationinterface_tests::unregister_validation_interface_race::test_method()::$_0&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:95
        [#6](/bitcoin-bitcoin/6/) 0x55abc8b9c94c in decltype(std::__invoke(_S_declval<0ul>())) std::thread::_Invoker<std::tuple<validationinterface_tests::unregister_validation_interface_race::test_method()::$_0> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/thread:244
        [#7](/bitcoin-bitcoin/7/) 0x55abc8b9c94c in std::thread::_Invoker<std::tuple<validationinterface_tests::unregister_validation_interface_race::test_method()::$_0> >::operator()() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/thread:253
        [#8](/bitcoin-bitcoin/8/) 0x55abc8b9c94c in std::thread::_State_impl<std::thread::_Invoker<std::tuple<validationinterface_tests::unregister_validation_interface_race::test_method()::$_0> > >::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/thread:196
        [#9](/bitcoin-bitcoin/9/) 0x7f8f0c6d4b2e  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbbb2e)
        [#10](/bitcoin-bitcoin/10/) 0x7f8f0cccefa2 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7fa2)
        [#11](/bitcoin-bitcoin/11/) 0x7f8f0c3a34ce in clone (/lib/x86_64-linux-gnu/libc.so.6+0xf94ce)
    
    Address 0x7f8f04f60dc0 is located in stack of thread T5 at offset 32 in frame
        [#0](/bitcoin-bitcoin/0/) 0x55abc8b9d1af in std::thread::_State_impl<std::thread::_Invoker<std::tuple<validationinterface_tests::unregister_validation_interface_race::test_method()::$_1> > >::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/thread:196
    
      This frame has 1 object(s):
        [32, 40) 'sub.i.i.i.i.i' <== Memory access at offset 32 is inside this variable
    HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
          (longjmp and C++ exceptions *are* supported)
    Thread T5 created by T0 here:
        [#0](/bitcoin-bitcoin/0/) 0x55abc7d1002d in pthread_create (/bitcoin/src/test/test_bitcoin+0x37702d)
        [#1](/bitcoin-bitcoin/1/) 0x7f8f0c6d4db4 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbbdb4)
        [#2](/bitcoin-bitcoin/2/) 0x55abc8b97b1e in validationinterface_tests::unregister_validation_interface_race_invoker() /bitcoin/src/test/validationinterface_tests.cpp:19:1
        [#3](/bitcoin-bitcoin/3/) 0x55abc7ded77f in boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:118:11
    
    SUMMARY: AddressSanitizer: unknown-crash /bitcoin/src/validationinterface.cpp:241:75 in CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&)::$_14::operator()(CValidationInterface&) const
    Shadow bytes around the buggy address:
      0x0ff2609e4160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e4170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e4180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e4190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e41a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    =>0x0ff2609e41b0: 00 00 00 00 00 00 00 00[00]00 00 00 00 00 00 00
      0x0ff2609e41c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e41d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e41e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e41f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      0x0ff2609e4200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    Shadow byte legend (one shadow byte represents 8 application bytes):
      Addressable:           00
      Partially addressable: 01 02 03 04 05 06 07 
      Heap left redzone:       fa
      Freed heap region:       fd
      Stack left redzone:      f1
      Stack mid redzone:       f2
      Stack right redzone:     f3
      Stack after return:      f5
      Stack use after scope:   f8
      Global redzone:          f9
      Global init order:       f6
      Poisoned by user:        f7
      Container overflow:      fc
      Array cookie:            ac
      Intra object redzone:    bb
      ASan internal:           fe
      Left alloca redzone:     ca
      Right alloca redzone:    cb
      Shadow gap:              cc
    Thread T4 created by T0 here:
        [#0](/bitcoin-bitcoin/0/) 0x55abc7d1002d in pthread_create (/bitcoin/src/test/test_bitcoin+0x37702d)
        [#1](/bitcoin-bitcoin/1/) 0x7f8f0c6d4db4 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbbdb4)
        [#2](/bitcoin-bitcoin/2/) 0x55abc8b97b1e in validationinterface_tests::unregister_validation_interface_race_invoker() /bitcoin/src/test/validationinterface_tests.cpp:19:1
        [#3](/bitcoin-bitcoin/3/) 0x55abc7ded77f in boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:118:11
    
    ==95792==ABORTING
    
  19. MarcoFalke commented at 11:47 AM on April 26, 2020: member

    cc @promag , @ryanofsky You seem qualified to review this?

  20. in src/rpc/mining.cpp:949 in bbbbf387d9 outdated
     947 | +    auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash());
     948 | +    RegisterSharedValidationInterface(sc);
     949 |      bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block);
     950 | -    UnregisterValidationInterface(&sc);
     951 | +    UnregisterSharedValidationInterface(sc);
     952 |      if (!new_block && accepted) {
    


    promag commented at 8:22 AM on April 27, 2020:

    An alternative is to call SyncWithValidationInterfaceQueue here.


    MarcoFalke commented at 11:36 AM on April 27, 2020:

    Alternative to what? The notification we are listening to is never put in the queue.


    promag commented at 11:52 AM on April 27, 2020:

    I mean that after UnregisterValidationInterface + SyncWithValidationInterfaceQueue there is no pointer to sc in validation queue.


    MarcoFalke commented at 11:54 AM on April 27, 2020:

    How does SyncWithValidationInterfaceQueue prevent new notifications from being scheduled for sc?


    promag commented at 12:02 PM on April 27, 2020:

    New notifications won't be scheduled because the UnregisterValidationInterface above. Only problem is a concurrent notification being called, so SyncWithValidationInterfaceQueue ensures that case doesn't happen.


    MarcoFalke commented at 12:19 PM on April 27, 2020:

    Now I understand. Yeah, correct.

  21. promag commented at 8:24 AM on April 27, 2020: member

    Code review ACK bbbbf387d9a35f095a80985810a461f87a21ebde.

    Like @ryanofsky suggested, we should only have RegisterValidationInterface(shared_ptr) so this is a step in that direction.

  22. ryanofsky commented at 1:46 PM on April 27, 2020: member

    Could you revert or move formatting changes in validationinterface.h/cpp files to a separate commit? I'm starting at that looking for actual changes but is it all reformatting?

  23. in src/rpc/mining.cpp:877 in bbbbf387d9 outdated
     873 | @@ -874,7 +874,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
     874 |      return result;
     875 |  }
     876 |  
     877 | -class submitblock_StateCatcher : public CValidationInterface
     878 | +class submitblock_StateCatcher final : public CValidationInterface
    


    ryanofsky commented at 1:48 PM on April 27, 2020:

    This is also unrelated cleanup?


    MarcoFalke commented at 2:23 PM on April 27, 2020:

    I don't know, the compiler told me to do it IIRC

  24. in src/test/validation_block_tests.cpp:207 in bbbbf387d9 outdated
     203 | @@ -204,14 +204,12 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
     204 |      for (auto& t : threads) {
     205 |          t.join();
     206 |      }
     207 | -    while (GetMainSignals().CallbacksPending() > 0) {
     208 | -        UninterruptibleSleep(std::chrono::milliseconds{100});
     209 | -    }
     210 | +    SyncWithValidationInterfaceQueue();
    


    ryanofsky commented at 1:49 PM on April 27, 2020:

    This is part of the bugfix or an optimization? Or a different bugfix?


    MarcoFalke commented at 2:24 PM on April 27, 2020:

    Thanks, split the optimization into a separate commit

  25. ryanofsky commented at 1:51 PM on April 27, 2020: member

    Concept ACK. This is a good find and all changes look ok, but there's so much going on it's a headache to reverse engineer everything that's happening. Suggest making one change per commit or perhaps dropping some of the changes

  26. ryanofsky commented at 1:55 PM on April 27, 2020: member

    It is also interesting if the bug described #18742#issue-407482998 started happening recently. Unless I missed something, it seems like the bug has been present a very long time, and I wonder if #18524 made it more likely to happen.

  27. MarcoFalke force-pushed on Apr 27, 2020
  28. MarcoFalke commented at 2:25 PM on April 27, 2020: member

    Addressed feedback by @ryanofsky. No code changes, just more commits.

  29. in src/test/validationinterface_tests.cpp:36 in fa9f3a5189 outdated
      31 | +
      32 | +    // Start thread to consume notifications
      33 | +    std::thread sub{[&] {
      34 | +        // keep going for about 1 sec, which is 250k iterations
      35 | +        for (int i = 0; i < 250000; i++) {
      36 | +            TestSubscriberNoop sub{};
    


    ryanofsky commented at 2:53 PM on April 27, 2020:

    In commit "test: Add unregister_validation_interface_race test" (fa9f3a51897d5457492f84f7c8a41fea783d687d)

    Note: This commit is (intentionally) adding a broken test. The test is broken because it registering a subscriber object that can go out of scope while events are still being sent.

    Might be worth saying in the commit message that if the test happens to fail during a git bisect, failure is expected and commit can be marked skip.


    MarcoFalke commented at 3:05 PM on April 27, 2020:

    Thanks. Fixed.

  30. ryanofsky approved
  31. ryanofsky commented at 2:58 PM on April 27, 2020: member

    Code review ACK fa33a632bf0240794561dbabb8fe14e999ac6504. Great find and changes are very clear now!

  32. MarcoFalke force-pushed on Apr 27, 2020
  33. MarcoFalke commented at 3:06 PM on April 27, 2020: member

    Force pushed again. Still no code changes, just more text in commit bodies.

  34. ryanofsky approved
  35. ryanofsky commented at 4:12 PM on April 27, 2020: member

    Code review ACK fa6d1a04ed91514bb70e1c861888ea50c2b30e93. Just commit message tweaks since last review

  36. MarcoFalke commented at 12:57 PM on April 28, 2020: member

    Open-Close to re-run ci. See #15847 (comment)

  37. MarcoFalke closed this on Apr 28, 2020

  38. MarcoFalke reopened this on Apr 28, 2020

  39. MarcoFalke commented at 1:06 PM on May 6, 2020: member

    @promag @fanquake Mind to re-ACK?

  40. jnewbery commented at 10:31 PM on May 13, 2020: member

    This commit is (intentionally) adding a broken test. The test is broken because it registering a subscriber object that can go out of scope while events are still being sent.

    If the test happens to fail (e.g. during a git bisect), failure is expected and commit can be marked skip.

    Why add a broken test to the git history? Can't you just add the test as the final commit?

  41. promag commented at 10:35 PM on May 13, 2020: member

    @jnewbery you have to rebase the test without the fix to see the test failing. What you suggest is simpler with functional tests - you can go back and forth on python tests while using the same binary.

    Maybe add [skip ci] to commit.

  42. promag commented at 10:48 PM on May 13, 2020: member

    This still doesn't prevent new cases right? I think the following would fix current cases and prevent new ones:

    @@ -129,12 +129,15 @@ void RegisterValidationInterface(CValidationInterface* callbacks)
    
     void UnregisterSharedValidationInterface(std::shared_ptr<CValidationInterface> callbacks)
     {
    -    UnregisterValidationInterface(callbacks.get());
    +    if (g_signals.m_internals) {
    +        g_signals.m_internals->Unregister(callbacks.get());
    +    }
     }
    
     void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
         if (g_signals.m_internals) {
             g_signals.m_internals->Unregister(pwalletIn);
    +        SyncWithValidationInterfaceQueue();
         }
     }
    
  43. jnewbery commented at 10:55 PM on May 13, 2020: member

    you have to rebase the test without the fix to see the test failing. @promag - yes, I understand that, but the guidance I've always heard is to add tests afterwards in order to not break git bisect. That seems like a good policy to me. I don't understand what's different about this PR than others.

  44. promag commented at 10:59 PM on May 13, 2020: member

    @jnewbery in this case it's the test that's fixed so what you suggest is to just add the file test and then explain how it could be broken.

  45. MarcoFalke commented at 11:50 PM on May 13, 2020: member

    I can put the test in /** ... */ in the first commit. This way:

    • People can still reproduce by removing the /** and */ and then running the test in a loop until it fails.
    • The diff in the last commit is applied to the test in the same way that it is applied to the miner, "proving" the fix is correct.
    • It doesn't break git bisect (well to some extent, as the bug will always exist in the functional tests in previous commit until the fix commit, and the unit test passes anyway most of the time with address sanitizer enabled and probably passes all the time without sanitizers enabled)
  46. test: Add unregister_validation_interface_race test
    This commit is (intentionally) adding a broken test. The test is broken
    because it registering a subscriber object that can go out of scope
    while events are still being sent.
    
    To run the broken test and reproduce the bug:
      - Remove comment /** and */
      - ./configure --with-sanitizers=address
      - export ASAN_OPTIONS=detect_leaks=0
      - make
      - while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no  ; do true; done
    fab6d060ce
  47. validationinterface: Rework documentation, Rename pwalletIn to callbacks fa770ce7fe
  48. test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue
    For the purpose of this test the two have the same outcome, but this one
    is shorter and avoids a sleep for 0.1 seconds.
    fa5ceb25fc
  49. miner: Avoid stack-use-after-return in validationinterface
    This is achieved by switching to a shared_ptr.
    
    Also, switch the validationinterfaces in the tests to use shared_ptrs
    for the same reason.
    7777f2a4bb
  50. MarcoFalke force-pushed on May 13, 2020
  51. MarcoFalke commented at 12:00 AM on May 14, 2020: member

    Pushed an empty diff to defuse the test. Added instructions to the commit message on how to add the fuse back in.

    $ git diff fa6d1a04ed 7777f2a4bb | wc
          0       0       0
    
  52. promag commented at 12:48 AM on May 14, 2020: member
  53. MarcoFalke commented at 10:57 AM on May 14, 2020: member

    @promag Yes, but that causes a deadlock on shutdown. Also the shutdown sequence should be bug free right now because the message handler is stopped before any unregisters should happen. I still like your idea, but I think it should be prepared, reviewed and merged for 0.21, not as a backport.

    For the backport we should aim at a fix that has little chance of breaking stuff. The fix here should satisfy this because the same fix has already been applied to the wallet in #18338, which is also in 0.20

  54. laanwj commented at 11:21 AM on May 14, 2020: member

    Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780

  55. promag commented at 11:43 AM on May 14, 2020: member

    Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780.

  56. fanquake merged this on May 14, 2020
  57. fanquake closed this on May 14, 2020

  58. MarcoFalke deleted the branch on May 14, 2020
  59. MarcoFalke commented at 12:44 PM on May 14, 2020: member

    For backport, only the first and last commit are needed.

  60. fanquake removed the label Needs backport (0.20) on May 14, 2020
  61. fanquake commented at 12:48 PM on May 14, 2020: member

    For backport, only the first and last commit are needed.

    Thanks. Will have this up in #18973 shortly.

  62. fjahr commented at 1:02 PM on May 14, 2020: member

    post merge code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780

  63. fanquake referenced this in commit a272174906 on May 14, 2020
  64. fanquake referenced this in commit 3b26cf855f on May 14, 2020
  65. fanquake referenced this in commit 37a620748b on May 15, 2020
  66. fanquake referenced this in commit cc7d34465b on May 15, 2020
  67. MarcoFalke referenced this in commit 17bdf2afae on May 15, 2020
  68. Fabcien referenced this in commit 5db69d6d38 on Jan 28, 2021
  69. deadalnix referenced this in commit 66c052d527 on Jan 28, 2021
  70. deadalnix referenced this in commit b3681986e3 on Jan 28, 2021
  71. backpacker69 referenced this in commit 2901f9eaa4 on Mar 28, 2021
  72. backpacker69 referenced this in commit 3c4a75a51a on Mar 28, 2021
  73. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-02 18:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me