Page MenuHomePhabricator

Move shared qt/bitcoind initialization steps to shared functions.
Needs RevisionPublic

Authored by schancel on Sun, Nov 25, 23:24.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Summary

Move shared initialization steps to their own functions. We eventually would like as
shared an initialization as possible so we can ensure that we can de-globalize
items such as the mempool.

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
move-shared-init
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4132
Build 6335: Bitcoin ABC Teamcity Staging
Build 6334: arc lint + arc unit

Event Timeline

schancel created this revision.Sun, Nov 25, 23:24
Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Nov 25, 23:24
Fabien added a subscriber: Fabien.Mon, Nov 26, 11:53
Fabien added inline comments.
src/bitcoind.cpp
178 ↗(On Diff #6098)

I don't know what the reason is for this comment, but the changes break this assumption.
Did you look at the impact of moving the data dir lock before deamonization ?

schancel marked an inline comment as done.Mon, Nov 26, 18:32
schancel added inline comments.
src/bitcoind.cpp
178 ↗(On Diff #6098)

I don't see how this matters at all. deamonization does a double fork, and will inherit the lock.

schancel updated this revision to Diff 6107.Mon, Nov 26, 19:45

Fix locking behavior when daemonizing

Fabien added inline comments.Mon, Nov 26, 20:31
src/bitcoind.cpp
169 ↗(On Diff #6107)

You may leave this block unchanged, I see no value it these changes. I you want to fix daemonization => daemonizing, please also refactor the other comments in order to have them consistent.

src/init.cpp
1763 ↗(On Diff #6107)

This should also be removed ?

schancel updated this revision to Diff 6114.Mon, Nov 26, 21:22
schancel marked 2 inline comments as done.

Fix comment

jasonbcox requested changes to this revision.Mon, Nov 26, 21:25
jasonbcox added a subscriber: jasonbcox.

I have a number of backports in my pipeline that touch this code. Please hold this diff until those are completed.

This revision now requires changes to proceed.Mon, Nov 26, 21:25