Skip to content

Regression(284012@main) Crash under DNSResolveQueueCFNet::performDNSLookup()#34783

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
cdumez:280987_DNSResolveQueueCFNet_crash
Oct 7, 2024
Merged

Regression(284012@main) Crash under DNSResolveQueueCFNet::performDNSLookup()#34783
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
cdumez:280987_DNSResolveQueueCFNet_crash

Conversation

@cdumez
Copy link
Contributor

@cdumez cdumez commented Oct 7, 2024

1255032

Regression(284012@main) Crash under DNSResolveQueueCFNet::performDNSLookup()
https://bugs.webkit.org/show_bug.cgi?id=280987
rdar://137067072

Reviewed by Per Arne Vollan.

284012@main introduced a timeout timer for DNS resolution in DNSResolveQueueCFNet::performDNSLookup().
When the timer would fire, it would cancel the DNS resolution by calling `nw_resolver_cancel()` and
THEN call the DNS resolution completion handler.

The issue is that the timer itself is owned by the block passed to nw_resolver_set_update_handler().
When calling `nw_resolver_cancel()`, this block would get destroyed, which in turns would destroy
the timeout timer and thus its lambda. Then, the timer lambda would try to call the completion
handler after it's already been deallocated.

* Source/WebCore/platform/network/cf/DNSResolveQueueCFNet.cpp:
(WebCore::DNSResolveQueueCFNet::performDNSLookup):

Canonical link: https://commits.webkit.org/284776@main

7b149e6

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 win-tests
✅ 🧪 webkitperl 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
🧪 vision-wk2 🧪 mac-intel-wk2
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv 🛠 mac-safer-cpp
🛠 tv-sim
🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez self-assigned this Oct 7, 2024
@cdumez cdumez added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Oct 7, 2024
Copy link
Contributor

@pvollan pvollan left a comment

Choose a reason for hiding this comment

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

R=me.

@cdumez cdumez added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 7, 2024
…ookup()

https://bugs.webkit.org/show_bug.cgi?id=280987
rdar://137067072

Reviewed by Per Arne Vollan.

284012@main introduced a timeout timer for DNS resolution in DNSResolveQueueCFNet::performDNSLookup().
When the timer would fire, it would cancel the DNS resolution by calling `nw_resolver_cancel()` and
THEN call the DNS resolution completion handler.

The issue is that the timer itself is owned by the block passed to nw_resolver_set_update_handler().
When calling `nw_resolver_cancel()`, this block would get destroyed, which in turns would destroy
the timeout timer and thus its lambda. Then, the timer lambda would try to call the completion
handler after it's already been deallocated.

* Source/WebCore/platform/network/cf/DNSResolveQueueCFNet.cpp:
(WebCore::DNSResolveQueueCFNet::performDNSLookup):

Canonical link: https://commits.webkit.org/284776@main
@webkit-commit-queue webkit-commit-queue force-pushed the 280987_DNSResolveQueueCFNet_crash branch from 7b149e6 to 1255032 Compare October 7, 2024 19:01
@webkit-commit-queue
Copy link
Collaborator

Committed 284776@main (1255032): https://commits.webkit.org/284776@main

Reviewed commits have been landed. Closing PR #34783 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 1255032 into WebKit:main Oct 7, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants