Skip to content

fix(edge): prevent crash on supernode resolution failure at startup#130

Open
catoc wants to merge 2 commits into
n42n:mainfrom
catoc:fix/supernode-dns-crash
Open

fix(edge): prevent crash on supernode resolution failure at startup#130
catoc wants to merge 2 commits into
n42n:mainfrom
catoc:fix/supernode-dns-crash

Conversation

@catoc

@catoc catoc commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Add a retry loop in edge_init to handle failed supernode DNS resolution. Prevents NULL pointer dereference of eee->curr_sn when no supernodes are available at startup.

#128

Add a retry loop in edge_init to handle failed supernode DNS resolution.
Prevents NULL pointer dereference of eee->curr_sn when no supernodes
are available at startup.
@hamishcoleman

Copy link
Copy Markdown
Contributor

Creating a loop with a sleep in it is architecturally the wrong solution.

Since the resolver can be run at any time during the operation of the vpn, there could be a scenario where the list again ends up empty.

Thus the solution should look for the users of eee->curr_sn and avoid dereferencing the NULL

@catoc

catoc commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

​I appreciate the feedback. In this PR, I aimed to follow the principles of minimal intrusion and least modification.

​My reasoning is based on the current program logic: once the eee->supernodes list is initially established, the background resolver periodically updates curr_sn. If a resolution fails in a real-world environment—which is common—the mechanism maintains the previous valid value of curr_sn rather than clearing it.

​The fact that no issues have been reported for a long time suggests that this "black box" mechanism is stable once it's past the initialization phase.

Therefore, I chose to ensure the system starts correctly without an error, rather than refactoring the dereferencing logic across the entire codebase. This avoids increasing the "surface area" for potential regressions in a proven, mature logic.

@AndyChiang888

Copy link
Copy Markdown

looks good for me

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.

3 participants