Repository navigation
Conversation
…p is not undone proxy_tunnel.stop stopped the earner first. When the tunnel stop then failed, the next heartbeat found a ready tunnel beside a stopped earner and started the earner again, losing the stop. The tunnel now stops first; an earner left running without its tunnel has no way out and the heartbeat leaves it alone. Same fix as CashPilot-Desktop's stopProxied.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 26 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
==========================================
- Coverage 93.97% 93.96% -0.01%
==========================================
Files 74 74
Lines 15678 15678
==========================================
- Hits 14733 14732 -1
- Misses 945 946 +1
🚀 New features to boost your workflow:
|
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stopping a proxied copy, by hand or at the traffic cap, could be undone by the next heartbeat.
proxy_tunnel.stopstopped the earner first, then the tunnel. If the tunnel stop failed, the tunnel stayed up and ready beside a stopped earner.earner_actiontreats that state as a reboot or crash and starts the earner, so the stop was lost. The same window opened for a moment on every successful stop, between the two calls, whenever a heartbeat ran in it.The fix reverses the order: the tunnel stops first, then the earner. If the tunnel stop fails, nothing has stopped and the error reaches the UI. If the earner stop fails after the tunnel stopped, the earner has no way out, as the "It fails closed" section of docs/proxies.md describes and
scripts/proxy_leak_test.shproves.earner_actionleaves it alone because the tunnel is not running.Tests:
test_stop_takes_the_tunnel_before_its_earnerreplaces the test that pinned the old order.test_a_stop_that_fails_halfway_is_not_undone_by_the_heartbeatmakes each stop fail in turn and checks that the heartbeat would not start the earner. The tunnel-fails case fails on main.The same fix for Desktop is GeiserX/CashPilot-Desktop#262, from the review of Desktop #245, which ported this code.