Skip to content

fix: skip nftables cleanup when auto redirect was never started - #93

Open
sparkfade wants to merge 1 commit into
SagerNet:devfrom
sparkfade:fix/nftables-cleanup-unstarted
Open

sparkfade wants to merge 1 commit into
SagerNet:devfrom
sparkfade:fix/nftables-cleanup-unstarted

Conversation

@sparkfade

Copy link
Copy Markdown

Summary

  • autoRedirect.Close() unconditionally deleted the inet sing-box nftables table, even for instances that never entered Start() and therefore never created it. Closing such an instance silently removes the table that is still managed by the running instance.
  • This is easy to hit: sing-box tools fetch (and the other tools subcommands) builds the box with box.New() + PreStart() and closes it on exit, so running a diagnostic tool against a config with auto_redirect breaks transparent proxying for the whole LAN until the service is restarted. (Observed on Linux with sing-box 1.14.2: the table disappears immediately after the command exits.)
  • The instance now marks the table as owned right before it deletes/recreates it in Start(), so failed setups are still cleaned up, and Close() only cleans up when it was marked. This mirrors the iptables backend, where cleanup only runs when the paths were configured during Start().

Test plan

  • netns: an instance started with tun + auto_redirect creates the table; a second instance that only runs PreStart/Close no longer deletes it
  • normal start/stop still creates and deletes the table
  • Start() failure after taking over the table still cleans up the partial table

@sparkfade
sparkfade force-pushed the fix/nftables-cleanup-unstarted branch from 6ee0bca to 4080cfa Compare September 29, 2026 06:48
@sparkfade
sparkfade force-pushed the fix/nftables-cleanup-unstarted branch 2 times, most recently from 6e6160d to 17fc1f1 Compare October 2, 2026 09:01
@sparkfade

Copy link
Copy Markdown
Author

@nekohasekai plz take a look at this thank u

@sparkfade
sparkfade force-pushed the fix/nftables-cleanup-unstarted branch from 944419e to 0e74bbd Compare October 9, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant