Repository navigation
Bump c-ares 1.19.1 to 1.34.6 - #85
Merged
Merged
Conversation
glbrntt
requested changes
Jun 30, 2026
glbrntt
left a comment
glbrntt
left a comment
Contributor
There was a problem hiding this comment.
Thanks for opening this, unfortunately it doesn't build on Linux though so you'll need to figure those build issues out.
Contributor
Author
Thanks! Fixed. The Verified on macOS and Ubuntu 22.04 / Swift 6.1. |
glbrntt
requested changes
Jul 13, 2026
glbrntt
left a comment
glbrntt
left a comment
Contributor
There was a problem hiding this comment.
Now that your other PR has been merged you'll need to update this PR to incorporate those changes.
Contributor
Author
@glbrntt , Done, I've merged main into this branch and resolved the conflicts. |
glbrntt
requested changes
Jul 15, 2026
josephnoir
pushed a commit
that referenced
this pull request
Oct 30, 2026
### Motivation Fixes #90. A query that fails synchronously crashes instead of throwing. Our iOS app hit this three times across two devices (iOS 26.4.2 and 26.7.1): twice as SIGSEGV in swift_release at DNSResolver_c-ares.swift:209, once as a libmalloc invalid-free abort at :210. All three had the same stack through Ares.query, and all occurred when a device lost its route to the configured resolvers. `Ares.query` frees the `QueryReplyHandler` box when `ares_query_dnsrec` returns a non-success status, on the premise stated in the comment: *"On failure the callback will not fire."* That premise doesn't hold — c-ares invokes the callback **and** returns the error on every reachable synchronous failure path: ```c if (ares_slist_len(channel->servers) == 0) { callback(arg, ARES_ENOSERVER, 0, NULL); // ares_send.c:123 return ARES_ENOSERVER; // ares_send.c:124 } ``` Same shape at `ares_send.c:141-142`, `:162-163` and `:172-174`. The callback's own `defer` (`DNSResolver_c-ares.swift:184-187`) has already released the handler and deallocated the box, so the cleanup on the error return is a second release of freed memory. This is a regression in 0.8.0 (`1bd427a`, #85), which introduced the block as part of moving from the deprecated `ares_query` to `ares_query_dnsrec`. The void-returning `ares_query` had no synchronous status to misinterpret. ### Modifications Removed the `deinitialize`/`deallocate` pair from the `status != ARES_SUCCESS` branch and corrected the comment. `handler.handle(status:dnsrec:)` is deliberately kept. `ares_query_dnsrec` returns `ARES_EFORMERR` without calling the callback when the channel is null (`ares_query.c:123-125`), and that is the only path where this call is what resumes the continuation — removing it would turn the crash into a hang. On the paths that do call back it is a no-op, since `QueryReplyHandler` enforces resume-once by clearing `resume` to `nil` under its lock. Added `CAresSynchronousFailureTests`. ### Result A synchronous failure from `ares_query_dnsrec` surfaces as a thrown `AsyncDNSResolver.Error` instead of terminating the process. No change to the success path or to asynchronous failures. ### Test Plan The new test reaches a synchronous failure offline, with no network, routing changes or privileges. An empty server list is joined into an empty CSV and passed to `ares_set_servers_ports_csv`, which treats it as "blank all servers" and succeeds (`ares_update_servers.c:1280-1286`), leaving the channel with none — so every subsequent query takes the `ares_slist_len(channel->servers) == 0` path above. It repeats 50 times, because a single over-release can land on memory that hasn't been reused and pass unnoticed. Without the patch, the test target aborts on the first test: ``` Test Case '-[AsyncDNSResolverTests.CAresSynchronousFailureTests test_synchronousFailure_throwsAndDoesNotDoubleFreeTheQueryHandler]' started. error: Process '.../xctest ...' exited with unexpected signal code 6 ``` Signal 6 is libmalloc's invalid-free check on the second `deallocate`. With the patch, both tests pass. Two of the three reported crashes took the other route — SIGSEGV in swift_release — because the freed box had already been reused, so the first deinitialize read a recycled pointer and followed it. The third was the same abort this test produces. Which symptom appears depends only on allocator timing. Our app worked around this by switching to the dnssd backend, so the patched c-ares path hasn't been exercised in production — the evidence for the fix itself is the test above. --------- Co-authored-by: EnzoHerrera <enzoherrera@cattlekrush.com>
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.
Summary
Updates the vendored c-ares C library from 1.19.1 to 1.34.6 (latest stable), picking up ~15 minor releases of bug fixes, security hardening, and protocol support.
Changes
Submodule
Sources/CAsyncDNSResolver/c-ares→v1.34.6Build integration (
Package.swift)src/libinto subdirectories (record/,str/,util/,dsa/,event/,legacy/,thirdparty/, and an internalsrc/lib/include/). SwiftPM globs sources recursively, so this needed:./c-ares/src/lib/includeto the header search path.excludelist for the two new non-source files.Config headers (cross-platform)
ares_config.h/ares_build.hregenerated for 1.34.6 (hand-vendored, since SwiftPM can't run c-ares's cmake step), keeping the repo's existing#if defined(_WIN32)→config-win32.hsplit.main's lowest-common-denominator convention. Concretely this fixes:AvailabilityMacros.hnot found on Linux (ares_rand.c).memmemundeclared on Linux/glibc (ares_str.c) — falls back to c-ares's portable implementation.sys/random.hnot found on newer macOS SDKs — falls back to/dev/urandom.ares_status_tmigrationc-ares 1.20+ moved the status constants from
int#defines into a realares_status_tenum (a distinct Swift type). Bridged via anares_status_t(_ status: Int32)initializer so the readablecase ARES_SUCCESS:labels stay intact; behavior unchanged.Deprecated-API migration (
DNSResolver_c-ares.swift,AresChannel.swift)1.34 deprecates
ares_query,ares_getsock, and allares_parse_*_replyfunctions. Migrated to the modern API:ares_query→ares_query_dnsrec. Its callback delivers an already-parsedares_dns_record_t, so the 10 reply parsers now read fields via theares_dns_record_rr_*/ares_dns_rr_get_*accessors instead ofares_parse_*_reply. Each parser filters the answer section by record type (the old API filtered internally). This also removes per-parser buffer allocation and a pre-existinghostent/dataleak.ares_query_dnsrecreports a status; on failure the callback doesn't fire, so the handler is freed and the continuation failed at the call site.ares_getsock→ARES_OPT_SOCK_STATE_CB.AresChannelowns aSocketRegistrythat the socket-state callback keeps current;QueryProcessorpolls it to driveares_process_fd. The registry's lifetime is bracketed by the channel so it stays valid acrossares_destroy.onCancel) still usesares_cancel, which is not deprecated.