random: mark RandAddPeriodic and SeedPeriodic as noexcept #17507

1. 
   fanquake commented at 3:27 pm on November 18, 2019: member

   The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was removed in #17270, meaning it, and its users can now be marked noexcept.

   This also corrects the docs in random.h for some of the changes in #17270.

2. doc: correct random.h docs after #17270 461e547877
3. random: mark RandAddPeriodic and SeedPeriodic as noexcept

   The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was
   removed in #17270, meaning it, and its users can now be marked noexcept.

   55b2cb199c
4. fanquake added the label Utils/log/libs on Nov 18, 2019
5. fanquake requested review from sipa on Nov 18, 2019
6. in src/random.cpp:453 in 55b2cb199c

   459- * this sleeping logic, so they are noexcept. The same is true for all the
   460- * GetRand*() functions that use GetRandBytes() indirectly.
   461- *
   462- * TODO: After moving away from interruptible boost-based thread management,
   463- * everything can become noexcept here.
   464+ * None of the RNG code should ever throw any exception.
   

   ---

   ---

   

   laanwj commented at 3:35 pm on November 18, 2019:

   Very good :+1:
7. 
   practicalswift commented at 3:49 pm on November 18, 2019: contributor

   ACK 55b2cb199c276781b6daa5438af2da57dea3ac52

   Very nice!

   Somewhat related: it would be nice to have the same explicit guarantee for LogPrint/LogPrintf. That would allow compilers and static analyzers to treat a larger parts of our code base as “non-throwing” instead of “potentially throwing” :)

8. 
   laanwj commented at 4:32 pm on November 18, 2019: member

   Somewhat related: it would be nice to have the same explicit guarantee for LogPrint/LogPrintf.

   Wouldn’t that be easy to do? LogPrintf already catches the only relevant exception, tinyformat::format_error, internally. I don’t think any exceptions are supposed to leak out of it.

9. 
   practicalswift commented at 4:42 pm on November 18, 2019: contributor

   @laanwj Yes, exactly! It is only a matter of formalizing that with a noexcept :)
10. 
    MarcoFalke commented at 5:26 pm on November 18, 2019: member

    @practicalswift Wouldn’t upstream be a better place to propose this change?
11. 
    practicalswift commented at 5:37 pm on November 18, 2019: contributor

    @MarcoFalke Upstream throws tinyformat::format_error which we catch: thus noexcept only makes sense on our end of things, no? :)
12. 
    MarcoFalke commented at 5:55 pm on November 18, 2019: member

    Fine, but LogPrintStr is also called, which might throw exceptions. In either case, this discussion should happen elsewhere.
13. 
    sipa commented at 7:27 pm on November 18, 2019: member

    ACK 55b2cb199c276781b6daa5438af2da57dea3ac52
14. 
    laanwj commented at 7:50 am on November 19, 2019: member

    I don’t think LogPrintStr is supposed to ever raise an exception, but sure, that’d need to be checked. Agree this is off topic here.

    ACK 55b2cb199c276781b6daa5438af2da57dea3ac52

15. 
    practicalswift commented at 9:30 am on November 19, 2019: contributor

    @laanwj @MarcoFalke Please note that SeedPeriodic which is annotated noexcept in this PR calls LogPrintf:

    https://github.com/bitcoin/bitcoin/blob/2065ef66ee6fb2b7bb442274f860813cad85b08c/src/random.cpp#L480-L491

    Thus the question of a noexcept annotation for LogPrintf is not entirely off topic here :)

16. 
    laanwj commented at 10:07 am on November 19, 2019: member

    Thus the question of a noexcept annotation for LogPrintf is not entirely off topic here :)

    Sure. I’m going to work on a PR to make logging noexcept.

17. 
    DrahtBot commented at 6:10 am on December 5, 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:

    • #17670 (Move events_hasher into RNGState() by sipa)

    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.

18. 
    sipa commented at 8:41 am on December 5, 2019: member

    ReACK. I accidentally redid part of this in #17670. The documentation improvements here should be done anyway, so let’s do this PR first.
19. laanwj referenced this in commit 6fff333c9f on Dec 5, 2019
20. laanwj merged this on Dec 5, 2019
21. laanwj closed this on Dec 5, 2019

    ---

22. fanquake deleted the branch on Dec 5, 2019
23. sidhujag referenced this in commit 9189c75128 on Dec 6, 2019
24. deadalnix referenced this in commit fce972a3a0 on May 22, 2020
25. sidhujag referenced this in commit 23b2206654 on Nov 10, 2020
26. random-zebra referenced this in commit 93f43f0f81 on Apr 14, 2021
27. DrahtBot locked this on Dec 16, 2021

Contributors
fanquake laanwj practicalswift MarcoFalke sipa DrahtBot

Labels
Utils/log/libs