rpc: Serialize in getblock without cs_main #15932

pull MarcoFalke wants to merge 3 commits into bitcoin:master from MarcoFalke:1905-rpcBlockNoLock changing 2 files +33 −15

MarcoFalke commented at 4:23 PM on May 1, 2019: member

No need to hold cs_main when serializing a struct to json

Fixes: #15925
rpc: Use IsValidNumArgs in getblock fa1c3591ad
fanquake added the label RPC/REST/ZMQ on May 1, 2019
rpc: Serialize in getblock without cs_main fab00a5cb9
MarcoFalke force-pushed on May 1, 2019
jnewbery commented at 1:50 PM on May 2, 2019: member

Does it make sense to add an AssertLockNotHeld(cs_main) to those functions?
promag commented at 2:10 PM on May 2, 2019: member

For reference #12153 (comment).

Concept ACK.

Does it make sense to add an AssertLockNotHeld(cs_main) to those functions?

Maybe EXCLUDES annotation instead?
laanwj commented at 2:20 PM on May 2, 2019: member

Concept ACK. The whole point of moving the lock of cs_main inside the individual RPC functions was to allow optimizations such as this.
rpc: Add lock annotations to block{,header}ToJSON faea56400d
MarcoFalke commented at 6:33 PM on May 2, 2019: member

Added lock annotations as requested by @promag and @jnewbery
DrahtBot commented at 6:45 PM on May 2, 2019: 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:
- #14802 (rpc: faster getblockstats using BlockUndo data by FelixWeis)
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.
jnewbery commented at 7:00 PM on May 2, 2019: member

utACK faea56400d5578023133cf4d1c761cdeb0c3e3da
fanatid commented at 5:27 AM on May 3, 2019: contributor

Why not use ENTER_CRITICAL_SECTION & LEAVE_CRITICAL_SECTION instead LOCK?
promag commented at 6:20 AM on May 3, 2019: member

Because a scoped lock is more clear and "secure".
jonasschnelli commented at 6:48 AM on May 3, 2019: contributor

utACK faea56400d5578023133cf4d1c761cdeb0c3e3da
MarcoFalke merged this on May 3, 2019
MarcoFalke closed this on May 3, 2019
MarcoFalke referenced this in commit 94daebf327 on May 3, 2019
MarcoFalke deleted the branch on May 3, 2019
sidhujag referenced this in commit ce64be77f2 on May 3, 2019
in src/rpc/blockchain.h:14 in faea56400d
```
  11 | +#include <sync.h>
  12 | +
  13 | +#include <stdint.h>
  14 | +#include <vector>
  15 | +
  16 | +extern RecursiveMutex cs_main;
```
elichai commented at 11:15 AM on January 7, 2020:

@MarcoFalke Do you use RecursiveMutex instead of CCriticalSection on purpose?
fanquake referenced this in commit af05bd9e1e on Jan 15, 2020
deadalnix referenced this in commit bb7cea810c on Jun 1, 2020
deadalnix referenced this in commit 6cd4a39874 on Jun 1, 2020
deadalnix referenced this in commit 38c4f1868d on Jun 1, 2020
LarryRuane referenced this in commit 3d1d30d4bc on May 10, 2021
LarryRuane referenced this in commit a99bc82078 on May 10, 2021
Munkybooty referenced this in commit 1afe02bc6c on Oct 12, 2021
Munkybooty referenced this in commit f6e9396aa8 on Oct 13, 2021
Munkybooty referenced this in commit 3c6c44c938 on Oct 14, 2021
DrahtBot locked this on Feb 15, 2022