[GH-ISSUE #1615] CachingClient::cache uses an immutable &self #710

New issue

Closed

opened 2026-03-15 23:55:25 +03:00 by kerem · 2 comments

kerem commented

2026-03-15 23:55:25 +03:00

Owner

Originally created by @Noah-Wagner on GitHub (Jan 14, 2022).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1615

I found the following function where we insert new cache entries: caching_client.rs.

I am new to Rust and don't understand all of the idioms, but it is a little surprising to me that the &self for this function is immutable, as we appear to modify the CachingClient::lru_cache. While this compiles (due to Arc/Mutex mutability), I wonder if it would make sense to also make the &self here mutable to clearly document that we are mutating state on our class?

Originally created by @Noah-Wagner on GitHub (Jan 14, 2022). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1615 I found the following function where we insert new cache entries: [caching_client.rs]( https://github.com/bluejekyll/trust-dns/blob/main/crates/resolver/src/caching_client.rs#L465). I am new to Rust and don't understand all of the idioms, but it is a little surprising to me that the `&self` for this function is immutable, as we appear to modify the `CachingClient::lru_cache`. While this compiles (due to Arc/Mutex mutability), I wonder if it would make sense to also make the `&self` here mutable to clearly document that we are mutating state on our class?

kerem closed this issue

2026-03-15 23:55:30 +03:00

kerem commented

2026-03-15 23:55:36 +03:00

Author

Owner

@djc commented on GitHub (Jan 14, 2022):

It's a desirable property for the Resolver or AsyncResolver to have resolving methods take &self rather than &mut self so you can share a single resolver in an Arc and easily run queries from different threads or tasks. Conceptually it also makes sense that a Resolver is not mutated whenever you run a resolver query. However, caching somewhat breaks that concept -- therefore the cache, in this case in the form of the CachingClient, contains an inner Arc<Mutex<_>> to make sure that the outward appearance of immutability isn't "broken".

Does that make sense?

@djc commented on GitHub (Jan 14, 2022): It's a desirable property for the `Resolver` or `AsyncResolver` to have resolving methods take `&self` rather than `&mut self` so you can share a single resolver in an `Arc` and easily run queries from different threads or tasks. Conceptually it also makes sense that a `Resolver` is not mutated whenever you run a resolver query. However, caching somewhat breaks that concept -- therefore the cache, in this case in the form of the `CachingClient`, contains an inner `Arc<Mutex<_>>` to make sure that the outward appearance of immutability isn't "broken". Does that make sense?

kerem commented

2026-03-15 23:55:41 +03:00

Author

Owner

@Noah-Wagner commented on GitHub (Jan 14, 2022):

This makes sense, thanks!

@Noah-Wagner commented on GitHub (Jan 14, 2022): This makes sense, thanks!

kerem referenced this issue

2026-03-16 02:16:31 +03:00

[PR #710] [CLOSED] Bump h2 from 0.1.16 to 0.1.17 #1624