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

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

MarcoFalke added the label Mining on Apr 22, 2020

MarcoFalke added the label Needs backport (0.19) on Apr 22, 2020

MarcoFalke added the label Needs backport (0.18) on Apr 22, 2020

MarcoFalke added the label Needs backport (0.20) on Apr 22, 2020

MarcoFalke added this to the milestone 0.20.0 on Apr 22, 2020

DrahtBot commented at 7:34 pm on April 22, 2020: member

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

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.

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? :)

MarcoFalke commented at 8:39 pm on April 22, 2020: member

  0118/145 - rpc_getblockstats.py failed, Duration: 3 s
  1
  2stdout:
  3
  42020-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
  5
  62020-04-15T15:10:12.198000Z TestFramework (ERROR): Unexpected exception caught during testing
  7
  8Traceback (most recent call last):
  9
 10  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 108, in _request
 11
 12    return self._get_response()
 13
 14  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 166, in _get_response
 15
 16    http_response = self.__conn.getresponse()
 17
 18  File "/usr/lib/python3.6/http/client.py", line 1346, in getresponse
 19
 20    response.begin()
 21
 22  File "/usr/lib/python3.6/http/client.py", line 307, in begin
 23
 24    version, status, reason = self._read_status()
 25
 26  File "/usr/lib/python3.6/http/client.py", line 276, in _read_status
 27
 28    raise RemoteDisconnected("Remote end closed connection without"
 29
 30http.client.RemoteDisconnected: Remote end closed connection without response
 31
 32During handling of the above exception, another exception occurred:
 33
 34Traceback (most recent call last):
 35
 36  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 112, in main
 37
 38    self.run_test()
 39
 40  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_getblockstats.py", line 97, in run_test
 41
 42    self.load_test_data(test_data)
 43
 44  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_getblockstats.py", line 89, in load_test_data
 45
 46    self.nodes[0].submitblock(b)
 47
 48  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
 49
 50    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
 51
 52  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 142, in __call__
 53
 54    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
 55
 56  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 113, in _request
 57
 58    self.__conn.request(method, path, postdata, headers)
 59
 60  File "/usr/lib/python3.6/http/client.py", line 1254, in request
 61
 62    self._send_request(method, url, body, headers, encode_chunked)
 63
 64  File "/usr/lib/python3.6/http/client.py", line 1300, in _send_request
 65
 66    self.endheaders(body, encode_chunked=encode_chunked)
 67
 68  File "/usr/lib/python3.6/http/client.py", line 1249, in endheaders
 69
 70    self._send_output(message_body, encode_chunked=encode_chunked)
 71
 72  File "/usr/lib/python3.6/http/client.py", line 1036, in _send_output
 73
 74    self.send(msg)
 75
 76  File "/usr/lib/python3.6/http/client.py", line 974, in send
 77
 78    self.connect()
 79
 80  File "/usr/lib/python3.6/http/client.py", line 946, in connect
 81
 82    (self.host,self.port), self.timeout, self.source_address)
 83
 84  File "/usr/lib/python3.6/socket.py", line 724, in create_connection
 85
 86    raise err
 87
 88  File "/usr/lib/python3.6/socket.py", line 713, in create_connection
 89
 90    sock.connect(sa)
 91
 92ConnectionRefusedError: [Errno 111] Connection refused
 93
 942020-04-15T15:10:12.261000Z TestFramework (INFO): Stopping nodes
 95
 962020-04-15T15:10:12.261000Z TestFramework.node0 (ERROR): Unable to stop node.
 97
 98Traceback (most recent call last):
 99
100  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
101
102    self.stop(wait=wait)
103
104  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
105
106    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
107
108  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 142, in __call__
109
110    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
111
112  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 107, in _request
113
114    self.__conn.request(method, path, postdata, headers)
115
116  File "/usr/lib/python3.6/http/client.py", line 1254, in request
117
118    self._send_request(method, url, body, headers, encode_chunked)
119
120  File "/usr/lib/python3.6/http/client.py", line 1265, in _send_request
121
122    self.putrequest(method, url, **skips)
123
124  File "/usr/lib/python3.6/http/client.py", line 1118, in putrequest
125
126    raise CannotSendRequest(self.__state)
127
128http.client.CannotSendRequest: Request-sent
129
130[node 0] Cleaning up leftover process
131
132stderr:
133
134Traceback (most recent call last):
135
136  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/rpc_getblockstats.py", line 165, in <module>
137
138    GetblockstatsTest().main()
139
140  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 135, in main
141
142    exit_code = self.shutdown()
143
144  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 242, in shutdown
145
146    self.stop_nodes()
147
148  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
149
150    node.stop_node(wait=wait)
151
152  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
153
154    raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
155
156AssertionError: Unexpected stderr =================================================================
157
158==3550==ERROR: AddressSanitizer: stack-use-after-return on address 0x7fdbfbb7dc40 at pc 0x5567a688f34f bp 0x7fdbfee66f10 sp 0x7fdbfee66f08
159
160READ of size 8 at 0x7fdbfbb7dc40 thread T4 (b-httpworker.0)
161
162    [#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
163
164    [#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
165
166    [#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
167
168    [#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
169
170    [#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
171
172    [#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
173
174    [#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
175
176    [#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
177
178    [#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
179
180    [#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
181
182    [#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
183
184    [#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
185
186    [#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
187
188    [#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
189
190    [#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
191
192    [#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
193
194    [#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
195
196    [#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
197
198    [#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
199
200    [#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
201
202    [#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
203
204    [#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
205
206    [#22](/bitcoin-bitcoin/22/) 0x7fdc0a3a76de  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbd6de)
207
208    [#23](/bitcoin-bitcoin/23/) 0x7fdc0b3436da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
209
210    [#24](/bitcoin-bitcoin/24/) 0x7fdc0965888e in clone (/lib/x86_64-linux-gnu/libc.so.6+0x12188e)
211
212Address 0x7fdbfbb7dc40 is located in stack of thread T6 (b-httpworker.2) at offset 3136 in frame
213
214    [#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
215
216  This frame has 57 object(s):
217
218    [32, 33) 'ref.tmp.i'
219
220    [48, 192) 'ref.tmp' (line 898)
221
222    [256, 288) 'agg.tmp'
223
224    [320, 321) 'ref.tmp1' (line 898)
225
226    [336, 368) 'agg.tmp6'
227
228    [400, 401) 'ref.tmp7' (line 898)
229
230    [416, 440) 'agg.tmp16'
231
232    [480, 864) 'ref.tmp18' (line 898)
233
234    [928, 960) 'agg.tmp19'
235
236    [992, 993) 'ref.tmp20' (line 898)
237
238    [1008, 1048) 'agg.tmp29'
239
240    [1088, 1092) 'ref.tmp30' (line 898)
241
242    [1104, 1136) 'agg.tmp37'
243
244    [1168, 1169) 'ref.tmp38' (line 898)
245
246    [1184, 1216) 'agg.tmp47'
247
248    [1248, 1249) 'ref.tmp48' (line 898)
249
250    [1264, 1288) 'agg.tmp57'
251
252    [1328, 1360) 'agg.tmp64'
253
254    [1392, 1393) 'ref.tmp65' (line 898)
255
256    [1408, 1448) 'agg.tmp74'
257
258    [1488, 1520) 'agg.tmp81'
259
260    [1552, 1553) 'ref.tmp82' (line 898)
261
262    [1568, 1600) 'agg.tmp91'
263
264    [1632, 1633) 'ref.tmp92' (line 898)
265
266    [1648, 1672) 'agg.tmp101'
267
268    [1712, 1713) 'ref.tmp108' (line 898)
269
270    [1728, 1752) 'agg.tmp117'
271
272    [1792, 1928) 'agg.tmp118'
273
274    [2000, 2032) 'agg.tmp119'
275
276    [2064, 2065) 'ref.tmp120' (line 898)
277
278    [2080, 2112) 'agg.tmp129'
279
280    [2144, 2145) 'ref.tmp130' (line 898)
281
282    [2160, 2184) 'agg.tmp139'
283
284    [2224, 2256) 'agg.tmp150'
285
286    [2288, 2320) 'agg.tmp151'
287
288    [2352, 2384) 'ref.tmp152' (line 898)
289
290    [2416, 2448) 'ref.tmp153' (line 898)
291
292    [2480, 2481) 'ref.tmp154' (line 898)
293
294    [2496, 2528) 'ref.tmp165' (line 898)
295
296    [2560, 2561) 'ref.tmp166' (line 898)
297
298    [2576, 2608) 'ref.tmp181' (line 898)
299
300    [2640, 2672) 'ref.tmp182' (line 898)
301
302    [2704, 2705) 'ref.tmp183' (line 898)
303
304    [2720, 2752) 'ref.tmp194' (line 898)
305
306    [2784, 2785) 'ref.tmp195' (line 898)
307
308    [2800, 2816) 'blockptr' (line 912)
309
310    [2832, 2864) 'ref.tmp326' (line 915)
311
312    [2896, 2897) 'ref.tmp327' (line 915)
313
314    [2912, 2944) 'ref.tmp369' (line 919)
315
316    [2976, 2977) 'ref.tmp370' (line 919)
317
318    [2992, 3024) 'hash' (line 922)
319
320    [3056, 3072) 'criticalblock37' (line 924)
321
322    [3088, 3104) 'criticalblock38' (line 937)
323
324    [3120, 3121) 'new_block' (line 944)
325
326    [3136, 3256) 'sc' (line 945) <== Memory access at offset 3136 is inside this variable
327
328    [3296, 3328) 'ref.tmp469' (line 945)
329
330    [3360, 3376) 'agg.tmp491'
331
332HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
333
334      (longjmp and C++ exceptions *are* supported)
335
336Thread T6 (b-httpworker.2) created by T0 here:
337
338    [#0](/bitcoin-bitcoin/0/) 0x5567a5fbe02d in pthread_create (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoind+0x1bc202d)
339
340    [#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)
341
342    [#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
343
344    [#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
345
346SUMMARY: 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
347
348Shadow bytes around the buggy address:
349
350  0x0ffbff767b30: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
351
352  0x0ffbff767b40: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
353
354  0x0ffbff767b50: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
355
356  0x0ffbff767b60: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
357
358  0x0ffbff767b70: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
359
360=>0x0ffbff767b80: f5 f5 f5 f5 f5 f5 f5 f5[f5]f5 f5 f5 f5 f5 f5 f5
361
362  0x0ffbff767b90: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
363
364  0x0ffbff767ba0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
365
366  0x0ffbff767bb0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
367
368  0x0ffbff767bc0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
369
370  0x0ffbff767bd0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
371
372Shadow byte legend (one shadow byte represents 8 application bytes):
373
374  Addressable:           00
375
376  Partially addressable: 01 02 03 04 05 06 07 
377
378  Heap left redzone:       fa
379
380  Freed heap region:       fd
381
382  Stack left redzone:      f1
383
384  Stack mid redzone:       f2
385
386  Stack right redzone:     f3
387
388  Stack after return:      f5
389
390  Stack use after scope:   f8
391
392  Global redzone:          f9
393
394  Global init order:       f6
395
396  Poisoned by user:        f7
397
398  Container overflow:      fc
399
400  Array cookie:            ac
401
402  Intra object redzone:    bb
403
404  ASan internal:           fe
405
406  Left alloca redzone:     ca
407
408  Right alloca redzone:    cb
409
410  Shadow gap:              cc
411
412Thread T4 (b-httpworker.0) created by T0 here:
413
414    [#0](/bitcoin-bitcoin/0/) 0x5567a5fbe02d in pthread_create (/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoind+0x1bc202d)
415
416    [#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)
417
418    [#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
419
420    [#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
421
422==3550==ABORTING !=

MarcoFalke removed the label Needs backport (0.18) on Apr 22, 2020

MarcoFalke removed the label Needs backport (0.19) on Apr 22, 2020

MarcoFalke commented at 8:54 pm on April 22, 2020: member

I might write a test tomorrow with steps to reproduce

MarcoFalke force-pushed on Apr 22, 2020

MarcoFalke force-pushed on Apr 23, 2020

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

0export ASAN_OPTIONS=detect_leaks=0
1make
2while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no  ; do true; done

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:

 0diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
 1index 11000774c0..6a311eeb44 100644
 2--- a/src/validationinterface.cpp
 3+++ b/src/validationinterface.cpp
 4@@ -11,6 +11,7 @@
 5 #include <primitives/block.h>
 6 #include <primitives/transaction.h>
 7 #include <scheduler.h>
 8+#include <util/time.h>
 9 
10 #include <future>
11 #include <unordered_map>
12@@ -80,6 +81,7 @@ public:
13             ++it->count;
14             {
15                 REVERSE_LOCK(lock);
16+                UninterruptibleSleep(std::chrono::microseconds(500));
17                 f(*it->callbacks);
18             }
19             it = --it->count ? std::next(it) : m_list.erase(it);

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:

 0/bitcoin# while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no  ; do true; done
 1Running 1 test case...
 2
 3*** No errors detected
 4Running 1 test case...
 5
 6*** No errors detected
 7Running 1 test case...
 8
 9*** No errors detected
10Running 1 test case...
11
12*** No errors detected
13Running 1 test case...
14=================================================================
15==95792==ERROR: AddressSanitizer: unknown-crash on address 0x7f8f04f60dc0 at pc 0x55abc9388623 bp 0x7f8f057777f0 sp 0x7f8f057777e8
16READ of size 8 at 0x7f8f04f60dc0 thread T4
17    [#0](/bitcoin-bitcoin/0/) 0x55abc9388622 in CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&)::$_14::operator()(CValidationInterface&) const /bitcoin/src/validationinterface.cpp:241:75
18    [#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
19    [#2](/bitcoin-bitcoin/2/) 0x55abc9388622 in CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&) /bitcoin/src/validationinterface.cpp:241
20    [#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
21    [#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
22    [#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
23    [#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
24    [#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
25    [#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
26    [#9](/bitcoin-bitcoin/9/) 0x7f8f0c6d4b2e  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbbb2e)
27    [#10](/bitcoin-bitcoin/10/) 0x7f8f0cccefa2 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7fa2)
28    [#11](/bitcoin-bitcoin/11/) 0x7f8f0c3a34ce in clone (/lib/x86_64-linux-gnu/libc.so.6+0xf94ce)
29
30Address 0x7f8f04f60dc0 is located in stack of thread T5 at offset 32 in frame
31    [#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
32
33  This frame has 1 object(s):
34    [32, 40) 'sub.i.i.i.i.i' <== Memory access at offset 32 is inside this variable
35HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
36      (longjmp and C++ exceptions *are* supported)
37Thread T5 created by T0 here:
38    [#0](/bitcoin-bitcoin/0/) 0x55abc7d1002d in pthread_create (/bitcoin/src/test/test_bitcoin+0x37702d)
39    [#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)
40    [#2](/bitcoin-bitcoin/2/) 0x55abc8b97b1e in validationinterface_tests::unregister_validation_interface_race_invoker() /bitcoin/src/test/validationinterface_tests.cpp:19:1
41    [#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
42
43SUMMARY: AddressSanitizer: unknown-crash /bitcoin/src/validationinterface.cpp:241:75 in CMainSignals::BlockChecked(CBlock const&, BlockValidationState const&)::$_14::operator()(CValidationInterface&) const
44Shadow bytes around the buggy address:
45  0x0ff2609e4160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
46  0x0ff2609e4170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
47  0x0ff2609e4180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
48  0x0ff2609e4190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
49  0x0ff2609e41a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50=>0x0ff2609e41b0: 00 00 00 00 00 00 00 00[00]00 00 00 00 00 00 00
51  0x0ff2609e41c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
52  0x0ff2609e41d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
53  0x0ff2609e41e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
54  0x0ff2609e41f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
55  0x0ff2609e4200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
56Shadow byte legend (one shadow byte represents 8 application bytes):
57  Addressable:           00
58  Partially addressable: 01 02 03 04 05 06 07 
59  Heap left redzone:       fa
60  Freed heap region:       fd
61  Stack left redzone:      f1
62  Stack mid redzone:       f2
63  Stack right redzone:     f3
64  Stack after return:      f5
65  Stack use after scope:   f8
66  Global redzone:          f9
67  Global init order:       f6
68  Poisoned by user:        f7
69  Container overflow:      fc
70  Array cookie:            ac
71  Intra object redzone:    bb
72  ASan internal:           fe
73  Left alloca redzone:     ca
74  Right alloca redzone:    cb
75  Shadow gap:              cc
76Thread T4 created by T0 here:
77    [#0](/bitcoin-bitcoin/0/) 0x55abc7d1002d in pthread_create (/bitcoin/src/test/test_bitcoin+0x37702d)
78    [#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)
79    [#2](/bitcoin-bitcoin/2/) 0x55abc8b97b1e in validationinterface_tests::unregister_validation_interface_race_invoker() /bitcoin/src/test/validationinterface_tests.cpp:19:1
80    [#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
81
82==95792==ABORTING

MarcoFalke commented at 11:47 am on April 26, 2020: member

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

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.

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.

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?

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

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

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

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.

MarcoFalke force-pushed on Apr 27, 2020

MarcoFalke commented at 2:25 pm on April 27, 2020: member

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

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.

ryanofsky approved

ryanofsky commented at 2:58 pm on April 27, 2020: member

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

MarcoFalke force-pushed on Apr 27, 2020

MarcoFalke commented at 3:06 pm on April 27, 2020: member

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

ryanofsky approved

ryanofsky commented at 4:12 pm on April 27, 2020: member

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

MarcoFalke commented at 12:57 pm on April 28, 2020: member

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

MarcoFalke closed this on Apr 28, 2020

MarcoFalke reopened this on Apr 28, 2020

MarcoFalke commented at 1:06 pm on May 6, 2020: member

@promag @fanquake Mind to re-ACK?

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?

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.

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:

 0@@ -129,12 +129,15 @@ void RegisterValidationInterface(CValidationInterface* callbacks)
 1
 2 void UnregisterSharedValidationInterface(std::shared_ptr<CValidationInterface> callbacks)
 3 {
 4-    UnregisterValidationInterface(callbacks.get());
 5+    if (g_signals.m_internals) {
 6+        g_signals.m_internals->Unregister(callbacks.get());
 7+    }
 8 }
 9
10 void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
11     if (g_signals.m_internals) {
12         g_signals.m_internals->Unregister(pwalletIn);
13+        SyncWithValidationInterfaceQueue();
14     }
15 }

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.

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.

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)

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

validationinterface: Rework documentation, Rename pwalletIn to callbacks fa770ce7fe

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

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

MarcoFalke force-pushed on May 13, 2020

MarcoFalke commented at 0: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.

0$ git diff fa6d1a04ed 7777f2a4bb | wc
1      0       0       0

promag commented at 0:48 am on May 14, 2020: member

@MarcoFalke did you see #18742 (comment)?

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

laanwj commented at 11:21 am on May 14, 2020: member

Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780

promag commented at 11:43 am on May 14, 2020: member

Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780.

fanquake merged this on May 14, 2020

fanquake closed this on May 14, 2020

MarcoFalke deleted the branch on May 14, 2020

MarcoFalke commented at 12:44 pm on May 14, 2020: member

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

fanquake removed the label Needs backport (0.20) on May 14, 2020

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.

fjahr commented at 1:02 pm on May 14, 2020: member

post merge code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780

fanquake referenced this in commit a272174906 on May 14, 2020

fanquake referenced this in commit 3b26cf855f on May 14, 2020

fanquake referenced this in commit 37a620748b on May 15, 2020

fanquake referenced this in commit cc7d34465b on May 15, 2020

MarcoFalke referenced this in commit 17bdf2afae on May 15, 2020

Fabcien referenced this in commit 5db69d6d38 on Jan 28, 2021

deadalnix referenced this in commit 66c052d527 on Jan 28, 2021

deadalnix referenced this in commit b3681986e3 on Jan 28, 2021

backpacker69 referenced this in commit 2901f9eaa4 on Mar 28, 2021

backpacker69 referenced this in commit 3c4a75a51a on Mar 28, 2021

DrahtBot locked this on Feb 15, 2022

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

Conflicts