apple / swift-async-dns-resolver

A Swift library for asynchronous DNS requests, wrapping c-ares with Swift-friendly APIs and data structures.
Apache License 2.0
82 stars 12 forks source link

Fix continuation memory leak in Ares.query #31

Closed dieb closed 4 months ago

dieb commented 4 months ago

Motivation

29

Modifications

Frankly not too sure exactly why this fixes the leak (Swift noob here), but it may be that pointer.deallocate() only frees the pointer, and not the underlying initialized.

Result

QueryReplyHandler gets deallocated properly and so does the continuation that was leaking.

Leaks from this are gone when running A/AAAA queries.

Test Plan

Running Xcode leaks instrument in my app, some code examples in #29 .

yim-lee commented 4 months ago

@swift-server-bot add to allowlist

yim-lee commented 4 months ago

@swift-server-bot add to allowlist

yim-lee commented 4 months ago

@swift-server-bot add to allowlist

yim-lee commented 4 months ago

@swift-server-bot test this please

yim-lee commented 4 months ago

Something is wrong with the webhook so CI is not getting triggered. Will ask someone for help looking into it in the morning.

yim-lee commented 4 months ago

@swift-server-bot test this please

dieb commented 4 months ago

@dieb Would you be comfortable making similar changes to DNSSD.query and QueryReplyHandler as well? If not, it's ok.

Absolutely, thanks for the oppo. Should be a quick thing.

dieb commented 4 months ago

@yim-lee I made the change for DNSSD but I was unable to test locally, test_concurrency hangs. I'm also getting errors on CAresDNSResolverTests test_queryTXT and test_concurrency. Would rely on you folks to review if the last commit is appropriate.

yim-lee commented 4 months ago

but I was unable to test locally, test_concurrency hangs.

@dieb Does this happen even before these changes?

I'm also getting errors on CAresDNSResolverTests test_queryTXT and test_concurrency.

Are these errors due to no results?

dieb commented 4 months ago

Same errors are happening in main and 1f5d6f46ac17e74d335b8fba668e5228510dbb8e (before #30), so I'm guessing it's something to do with my local environment.

Running swift test directly in the command-line without the docker-compose thingy and I noticed only mDNSResponder shows up in ps | grep dns, not sure if I need anything else for DNSSD.

Edit: errors are connection refused and test_concurrency hanging.

Test log ``` ➜ swift-async-dns-resolver git:(main) ✗ swift test Building for debugging... [6/6] Linking swift-async-dns-resolverPackageTests Build complete! (0.72s) Test Suite 'All tests' started at 2024-02-21 14:31:48.762. Test Suite 'swift-async-dns-resolverPackageTests.xctest' started at 2024-02-21 14:31:48.762. Test Suite 'AresChannelTests' started at 2024-02-21 14:31:48.762. Test Case '-[AsyncDNSResolverTests.AresChannelTests test_init]' started. Test Case '-[AsyncDNSResolverTests.AresChannelTests test_init]' passed (0.001 seconds). Test Suite 'AresChannelTests' passed at 2024-02-21 14:31:48.763. Executed 1 test, with 0 failures (0 unexpected) in 0.001 (0.001) seconds Test Suite 'AresErrorTests' started at 2024-02-21 14:31:48.763. Test Case '-[AsyncDNSResolverTests.AresErrorTests test_initFromCode]' started. Test Case '-[AsyncDNSResolverTests.AresErrorTests test_initFromCode]' passed (0.000 seconds). Test Suite 'AresErrorTests' passed at 2024-02-21 14:31:48.764. Executed 1 test, with 0 failures (0 unexpected) in 0.000 (0.000) seconds Test Suite 'AresOptionsTests' started at 2024-02-21 14:31:48.764. Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_AresOptions_socketStateCallback]' started. Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_AresOptions_socketStateCallback]' passed (0.000 seconds). Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_flags]' started. Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_flags]' passed (0.000 seconds). Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_OptionsToAresOptions]' started. Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_OptionsToAresOptions]' passed (0.000 seconds). Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_rotate]' started. Test Case '-[AsyncDNSResolverTests.AresOptionsTests test_rotate]' passed (0.000 seconds). Test Suite 'AresOptionsTests' passed at 2024-02-21 14:31:48.765. Executed 4 tests, with 0 failures (0 unexpected) in 0.001 (0.001) seconds Test Suite 'CAresDNSResolverTests' started at 2024-02-21 14:31:48.765. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_concurrency]' started. /Users/dieb/Projects/Terran/OSS/swift-async-dns-resolver/Tests/AsyncDNSResolverTests/c-ares/CAresDNSResolverTests.swift:128: error: -[AsyncDNSResolverTests.CAresDNSResolverTests test_concurrency] : failed: caught error: "connection refused: " Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_concurrency]' failed (0.071 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryAAAA]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryAAAA]' passed (0.022 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryA]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryA]' passed (0.013 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryCNAME]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryCNAME]' passed (0.025 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryMX]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryMX]' passed (0.023 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryNAPTR]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryNAPTR]' passed (0.023 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryNS]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryNS]' passed (0.024 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryPTR]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryPTR]' passed (0.013 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_querySOA]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_querySOA]' passed (0.021 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_querySRV]' started. Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_querySRV]' passed (0.025 seconds). Test Case '-[AsyncDNSResolverTests.CAresDNSResolverTests test_queryTXT]' started. error: Exited with signal code 13 ```