Skip to content

rpcban fixes #6307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
laanwj merged 3 commits into from
Jun 19, 2015
Merged

rpcban fixes #6307

laanwj merged 3 commits into from
Jun 19, 2015

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Jun 19, 2015

  • fix a missing lock in clearban
  • add Ipv6 tests

@jonasschnelli jonasschnelli mentioned this pull request Jun 19, 2015
Merged
@laanwj
Copy link
Member

laanwj commented Jun 19, 2015

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
Copy link
Contributor Author

jonasschnelli commented Jun 19, 2015

Oh. Right. This is serious. I'm investigating...

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Jun 19, 2015

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
Copy link
Member

laanwj commented Jun 19, 2015

Tested ACK.
I would call this "fix" not "overhaul".

@jonasschnelli jonasschnelli changed the title rpcban overhaul rpcban fixes Jun 19, 2015
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Jun 19, 2015

Agreed. Changed PR title.

@laanwj laanwj merged commit 1c043d5 into bitcoin:master Jun 19, 2015
laanwj added a commit that referenced this pull request Jun 19, 2015
@laanwj
Merge pull request #6307
7ecdcd9 
1c043d5 fix lock issue for QT node diconnect and RPC disconnectnode (Jonas Schnelli)
932687b setban: add IPv6 tests (Jonas Schnelli)
62909f6 fix missing lock in CNode::ClearBanned() (Jonas Schnelli)
@str4d str4d mentioned this pull request Feb 14, 2017
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Mar 4, 2017
@zkbot
Auto merge of #2100 - str4d:2074-rpc, r=arcalinea
a7cf698 
Bitcoin 0.12 RPC PRs 1

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6266
- bitcoin/bitcoin#6257
- bitcoin/bitcoin#6271
- bitcoin/bitcoin#6158
- bitcoin/bitcoin#6307
- bitcoin/bitcoin#6290
- bitcoin/bitcoin#6262
- bitcoin/bitcoin#6088
- bitcoin/bitcoin#6339
- bitcoin/bitcoin#6299 (partial, remainder in #2099)
- bitcoin/bitcoin#6350
- bitcoin/bitcoin#6247
- bitcoin/bitcoin#6362
- bitcoin/bitcoin#5486
- bitcoin/bitcoin#6417
- bitcoin/bitcoin#6398 (partial, remainder was included in #1950)
- bitcoin/bitcoin#6444
- bitcoin/bitcoin#6456 (partial, remainder was included in #2082)
- bitcoin/bitcoin#6380
- bitcoin/bitcoin#6970

Part of #2074.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jonasschnelli @laanwj