rpcban fixes #6307

pull jonasschnelli wants to merge 3 commits into bitcoin:master from jonasschnelli:2015/06/rpcban_overhaul changing 4 files +31 −3

jonasschnelli commented at 11:52 AM on June 19, 2015: contributor
- fix a missing lock in clearban
- add Ipv6 tests
fix missing lock in CNode::ClearBanned() 62909f68a0
setban: add IPv6 tests 932687bc10
jonasschnelli cross-referenced this on Jun 19, 2015 from issue gitian: add a gitian-win-signer descriptor by theuni

laanwj commented at 1:21 PM on June 19, 2015: member

Travis error looks very serious, and occurs in the node handling test:

*** buffer overflow detected ***: /home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7faed05b9007]
/lib/x86_64-linux-gnu/libc.so.6(+0x107f00)[0x7faed05b7f00]
/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind(__fdelt_chk+0x22)[0x7faed1ce2d25]
/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind(+0x2315e3)[0x7faed1b115e3]
/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind(_Z11TraceThreadIPFvvEEvPKcT_+0x78)[0x7faed1b33f18]
/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind(+0x235adb)[0x7faed1b15adb]
/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind(+0x568294)[0x7faed1e48294]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x7e9a)[0x7faed0877e9a]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7faed05a24bd]
===== Memory map: ========

jonasschnelli commented at 1:25 PM on June 19, 2015: contributor

Oh. Right. This is serious. I'm investigating...
fix lock issue for QT node diconnect and RPC disconnectnode 1c043d5672
jonasschnelli commented at 2:58 PM on June 19, 2015: contributor

Added a commit that fixes a race condition during node disconnecting. Before this PR, we where using CNode::CloseSocketDisconnect() to disconnect nodes from the UI's peer table context menu as well over the rpc ban and disconnectnode functions. CNode::CloseSocketDisconnect() does close the socket directly, which is not thread safe because of missing locks around the socket handler.

This PR changes the disconnecting to go over the CNode::fDisconnect flag (which causes to disconnect the node during the existing node threads).
laanwj commented at 2:58 PM on June 19, 2015: member

Tested ACK. I would call this "fix" not "overhaul".
jonasschnelli renamed this:
~~rpcban overhaul~~
rpcban fixes
on Jun 19, 2015
jonasschnelli commented at 3:00 PM on June 19, 2015: contributor

Agreed. Changed PR title.
laanwj merged this on Jun 19, 2015
laanwj closed this on Jun 19, 2015
laanwj referenced this in commit 7ecdcd99ca on Jun 19, 2015
str4d cross-referenced this on Feb 14, 2017 from issue Bitcoin 0.12 RPC PRs 1 by str4d
zkbot referenced this in commit 9af55822fb on Feb 15, 2017
zkbot referenced this in commit a7cf698873 on Mar 4, 2017
bitcoin locked this on Sep 8, 2021

Contributors

jonasschnelli

laanwj

Linked (view graph)

#6303 gitian: add a gitian-win-signer descriptor