I’m reviewing the Tor controller’s reconnect-related code and noticed that the reconnect timeout had no limit. This could lead to excessively long delays.
This PR introduces a maximum reconnect timeout of 600 seconds (10 minutes) to prevent excessive delays in reconnection attempts. It also updates the log message to display the retry delay in whole seconds for better readability.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31979.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/38156083995
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
633@@ -631,10 +634,14 @@ void TorController::disconnected_cb(TorControlConnection& _conn)
634 if (!reconnect)
635 return;
636
637- LogDebug(BCLog::TOR, "Not connected to Tor control port %s, trying to reconnect\n", m_tor_control_center);
638-
639 // Single-shot timer for reconnect. Use exponential backoff.
640- struct timeval time = MillisToTimeval(int64_t(reconnect_timeout * 1000.0));
641+ int64_t timeout = std::min(reconnect_timeout, RECONNECT_TIMEOUT_MAX);
timeout should be a float, this a comparison on two float values returning a float.
645+ LogDebug(BCLog::TOR, "Not connected to Tor control port %s, retrying in %.2f seconds\n",
646+ m_tor_control_center, timeout);
647+
648 if (reconnect_ev)
649 event_add(reconnect_ev, &time);
650 reconnect_timeout *= RECONNECT_TIMEOUT_EXP;
imo it would be better to do the capping here:
0reconnect_timeout = std::min(reconnect_timeout * RECONNECT_TIMEOUT_EXP, RECONNECT_TIMEOUT_MAX);
Also please update the comment above to something like Single-shot timer for reconnect. Use exponential backoff with a maximum.
Concept ACK.
It’s debatable whether we need an exponential backoff here in the first place. After all Tor is generally a local service, there is no risk of creating a DoS. i added it back then, to reduce the amount of log spam in case Tor isn’t running. However we have a better logging system now that takes both category and level into account, so that motivation is no longer relevant.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/38169170491
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
Signed-off-by: Eval EXEC <execvy@gmail.com>
I compiled this branch, and launched bitcoind by bitcoind -debug=tor, start a tor-server and shutdown tor server, got:
0$ grep tor ~/.bitcoin/debug.log | grep retrying
12025-03-04T13:16:18Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 1.00 s
22025-03-04T13:16:19Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 1.50 s
32025-03-04T13:16:21Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 2.25 s
42025-03-04T13:16:23Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 3.38 s
52025-03-04T13:16:26Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 5.06 s
62025-03-04T13:16:31Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 7.59 s
72025-03-04T13:16:39Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 11.39 s
82025-03-04T13:16:50Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 17.09 s
92025-03-04T13:17:08Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 25.63 s
102025-03-04T13:17:33Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 38.44 s
112025-03-04T13:18:12Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 57.67 s
122025-03-04T13:19:09Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 86.50 s
132025-03-04T13:20:36Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 129.75 s
142025-03-04T13:22:46Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 194.62 s
152025-03-04T13:26:00Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 291.93 s
162025-03-04T13:30:53Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 437.89 s
172025-03-04T13:38:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
182025-03-04T13:48:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
192025-03-04T13:58:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
202025-03-04T14:08:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
212025-03-04T14:18:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
222025-03-04T14:28:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
232025-03-04T14:38:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
242025-03-04T14:48:11Z [tor] Not connected to Tor control port 127.0.0.1:9051, retrying in 600.00 s
