Skip to content

fix: replace Mutex<DnsState> with OnceCell in NonLocalDnsResolver#2449

Merged
svix-jplatte merged 4 commits into
svix:mainfrom
Rohan-Singla:fix/dns-resolver-oncecell-init
Jul 10, 2026
Merged

fix: replace Mutex<DnsState> with OnceCell in NonLocalDnsResolver#2449
svix-jplatte merged 4 commits into
svix:mainfrom
Rohan-Singla:fix/dns-resolver-oncecell-init

Conversation

@Rohan-Singla

@Rohan-Singla Rohan-Singla commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Motivation

NonLocalDnsResolver used Arc<Mutex<DnsState>> for lazy initialization of the DNS resolver. During the Init state, the mutex lock was held across an .await boundary specifically across the new_resolver().await call which performs I/O (reads system DNS config and builds the resolver).

This meant that under concurrent webhook dispatch, all DNS calls arriving before initialization completed would block waiting to acquire the mutex, serializing them behind a single I/O operation. On a high-throughput instance processing thousands of webhooks per second, this is a contention point at startup.

Solution

Replaced Arc<Mutex<DnsState>> with Arc<tokio::sync::OnceCell<TokioResolver>>. The get_or_try_init method handles the lazy initialization correctly only one caller runs the initializer, concurrent callers wait on a lightweight internal notifier (not a mutex), and all subsequent calls after initialization is complete are lock-free atomic loads.

The DnsState enum and the Arc wrapper around TokioResolver are both removed as they are no longer needed. new_resolver() now returns TokioResolver directly. No behavior change the DNS resolution logic and IP filtering are untouched.

tokio::sync::OnceCell requires no new dependency as tokio is already pulled in with features = ["full"].

@Rohan-Singla
Rohan-Singla requested a review from a team July 10, 2026 06:53
@Rohan-Singla
Rohan-Singla requested a review from a team as a code owner July 10, 2026 06:53

@svix-jplatte svix-jplatte left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I'm wondering why we ever made new_resolver async given that it does no async I/O. Would you mind also changing that function to do all its actual work in a blocking tokio task?

Comment thread server/svix-server/src/core/webhook_http_client.rs Outdated
Comment thread server/svix-server/src/core/webhook_http_client.rs Outdated
builder.build()
})
.await
.unwrap_or_else(|e| std::panic::resume_unwind(e.into_panic()))

@svix-jplatte svix-jplatte Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I hadn't encountered this pattern before, but it seems more sensible than a plain unwrap.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahan cool!

@svix-jplatte svix-jplatte left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again!

@svix-jplatte
svix-jplatte merged commit 77e421b into svix:main Jul 10, 2026
37 checks passed
@Rohan-Singla

Copy link
Copy Markdown
Contributor Author

Thanks again!

Welcome looking forward to contribute more!

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.

2 participants