Skip to content

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Jan 26, 2015

Rebased version of #5107.

  • update for RPC methods added since 84d13ee: setmocktime, invalidateblock, reconsiderblock. Only the first, setmocktime, required a change, the other two are thread safe.
  • remove no-longer-used threadSafe column from RPC table
  • remove duplicate invalidateblock and reconsiderblock from RPC table

CodeShark and others added 2 commits January 28, 2015 07:41
Rebased by @laanwj:

- update for RPC methods added since 84d13ee: setmocktime,
  invalidateblock, reconsiderblock. Only the first, setmocktime, required a change,
  the other two are thread safe.
- invalidateblock and reconsiderblock were defined doubly
- remove no-longer-used threadSafe, as locks have been pushed down
@laanwj laanwj force-pushed the 2015_01_rpc_pushdown_locks branch from 51a328a to 5ebe095 Compare January 28, 2015 06:42
@jonasschnelli
Copy link
Contributor

Tested ACK.

@jonasschnelli
Copy link
Contributor

I assume there is a minimal post PR overhaul of https://github.com/bitcoin/bitcoin/blob/master/src/rpcmining.cpp#L444 required. IMO not urgent.

@laanwj laanwj merged commit 5ebe095 into bitcoin:master Feb 4, 2015
laanwj added a commit that referenced this pull request Feb 4, 2015
5ebe095 Trim RPC command table (Wladimir J. van der Laan)
4401b2d Removed main.h dependency from rpcserver.cpp (Eric Lombrozo)
@sdaftuar
Copy link
Member

sdaftuar commented Feb 4, 2015

This commit appears to have broken the rpc-test getblocktemplate_longpoll.py. Relevant failure:

  File "./getblocktemplate_longpoll.py", line 66, in run_test
    assert(thr.is_alive())

I'm not familiar enough with the test yet to understand what is going on, but I verified the test broke at this commit and succeeds in the previous one.

@jonasschnelli
Copy link
Contributor

I can confirm that the test is broken.
Assertion failed: (!pthread_mutex_unlock(&m)), function unlock...

I think it has to do with the LEAVE_CRITICAL_SECTION.

jonasschnelli added a commit to jonasschnelli/bitcoin that referenced this pull request Feb 4, 2015
getblocktemplate didn't have a wallet lock before bitcoin#5711 and IMO there is no need for LEAVE/ENTER critical section.
@jonasschnelli
Copy link
Contributor

I don't see a need for https://github.com/bitcoin/bitcoin/blob/master/src/rpcmining.cpp#L451 LEAVE and ENTER crictial section.
But maybe i'm wrong.

random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Jul 1, 2020
9f4868a [move] move listunspent to wallet/rpcwallet.cpp (furszy)
e85e4f1 [RPC] Move RPC dispatch table registration to rpcwallet code (furszy)
c0bc348 [RPC] push down reqWallet [first step] (furszy)
cdb80a6 [RPC] Remove not used threadsafe in CRPCCommand (furszy)

Pull request description:

  Initial work over the RPC dispatch table registration update.

  Includes the following changes:

  1) Remove the unused threadSafe member from every CPRCCommand. (bitcoin#5711 , second commit)
  2) Remove reqWallet from every CPRCCommand (bitcoin#5992 , second commit partially).
  3) Move most of the wallet RPC commands registration to the rpcwallet file (bitcoin#7307 without the `EnsureWalletIsAvailable` on every RPC call --> mainly because the wallet RPC commands will only be registered if the wallet is available).

  to do: Move the remaining commands that were not moved from server to rpcwallet.

  ----------

  Next step would be bitcoin#7766 (If anyone want to tackle it, feel more than welcome to tackle it).

ACKs for top commit:
  Fuzzbawls:
    ACK 9f4868a
  random-zebra:
    All good. ACK 9f4868a and merging...

Tree-SHA512: cd83cdc2de81efb5cb84d39753a160b19d8ca2967a1b303d85c3279559b478352c5a4316942c7a654e9c667b5a8f5bfa18020c1b6c9e78f4c408028840d0fc86
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants