Skip to content

RELEASE ASSERT: http/tests/security/contentSecurityPolicy/multipart-three-part.py is a flaky crash#64646

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:314243_multipart_responses
May 13, 2026
Merged

RELEASE ASSERT: http/tests/security/contentSecurityPolicy/multipart-three-part.py is a flaky crash#64646
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:314243_multipart_responses

Conversation

@cdumez
Copy link
Copy Markdown
Contributor

@cdumez cdumez commented May 11, 2026

071edf4

RELEASE ASSERT: http/tests/security/contentSecurityPolicy/multipart-three-part.py is a flaky crash
https://bugs.webkit.org/show_bug.cgi?id=314243
rdar://169360779

Reviewed by Brent Fulgham and Brady Eidson.

http/tests/security/contentSecurityPolicy/multipart-three-part.py is
flaky because NetworkResourceLoader::didReceiveResponse() sometimes
overwrites m_responseCompletionHandler before the previous one has been
called, hitting the CompletionHandler destructor assertion.

For a multipart/x-mixed-replace main resource, the network layer can
deliver follow-up parts via didReceiveResponse() before the WebProcess
has answered ContinueDidReceiveResponse for the first part. The first
part's round-trip is async because it goes through
DocumentLoader::responseReceived() -> checkContentPolicy(), which IPCs
to the UIProcess. Assigning the new completion handler over the still-
pending one destroyed it without invoking it, asserting in
CompletionHandler's destructor.

Replace the single m_responseCompletionHandler with a Deque so each
incoming response queues its own handler, and each ContinueDidReceive-
Response IPC drains the front of the queue. This mirrors the WebProcess
flow (it processes each part in order, sending one ContinueDidReceive-
Response per approved part), so per-part validation is preserved. On
loader teardown / convertToDownload, any remaining handlers are drained
with PolicyAction::Ignore.

No new tests, covered by existing test.

* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::~NetworkResourceLoader):
(WebKit::NetworkResourceLoader::convertToDownload):
(WebKit::NetworkResourceLoader::transferToNewWebProcess):
(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::continueDidReceiveResponse):
* Source/WebKit/NetworkProcess/NetworkResourceLoader.h:

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

67dff1a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win loading 🛠 ios-apple
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 win-tests ⏳ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ⏳ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez self-assigned this May 11, 2026
@cdumez cdumez added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label May 11, 2026
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 12, 2026
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label May 12, 2026
@cdumez cdumez force-pushed the 314243_multipart_responses branch from 7c3e2a7 to f9b30dc Compare May 12, 2026 10:37
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 12, 2026
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label May 12, 2026
@cdumez cdumez force-pushed the 314243_multipart_responses branch from f9b30dc to 67dff1a Compare May 12, 2026 10:57
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 12, 2026
@brentfulgham brentfulgham removed the merging-blocked Applied to prevent a change from being merged label May 12, 2026
@brentfulgham
Copy link
Copy Markdown
Contributor

Looks like a Swift failure on watchOS:

mulator/usr/local/include/pal/ExportMacros.h:32:
#include <wtf/ExportMacros.h>
<module-includes>:38:9: note: in file included from <module-includes>:38:
#import "./AbstractRefCountedAndCanMakeWeakPtr.h"
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/./AbstractRefCountedAndCanMakeWeakPtr.h:29:10: note: in file included from /Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/./AbstractRefCountedAndCanMakeWeakPtr.h:29:
#include <wtf/CanMakeWeakPtr.h>
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/CanMakeWeakPtr.h:29:10: note: in file included from /Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/CanMakeWeakPtr.h:29:
#include <wtf/CompactRefPtrTuple.h>
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/CompactRefPtrTuple.h:28:10: note: in file included from /Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/CompactRefPtrTuple.h:28:
#include <wtf/CompactPointerTuple.h>
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/CompactPointerTuple.h:31:10: note: in file included from /Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/CompactPointerTuple.h:31:
#include <wtf/FastMalloc.h>
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/wtf/FastMalloc.h:23:10: error: could not build module 'bmalloc'
#include <bmalloc/BPlatform.h>
<module-includes>:1:9: note: in file included from <module-includes>:1:
#import "ExportMacros.h"
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/WebKitBuild/Release-watchsimulator/usr/local/include/pal/ExportMacros.h:32:10: error: could not build module 'wtf'
#include <wtf/ExportMacros.h>
/Volumes/Data/worker/watchOS-26-Simulator-Build-EWS/build/Source/WebCore/PAL/pal/crypto/CryptoKit+UnsafeOverlays.swift:26:8: error: could not build Objective-C module 'pal'
import pal.Core.crypto.CryptoTypes
SwiftCompile CryptoKit+UnsafeOverlays.swift
<unknown>:0: warning: unknown warning group: 'ForeignReferenceType' [#UnknownWarningGroup]
<unknown>:0: warning: unknown warning group: 'ForeignReferenceType' [#UnknownWarningGroup]
SwiftCompile CryptoKit+UnsafeOverlays.swift
CompilationCacheMetrics

Copy link
Copy Markdown
Contributor

@brentfulgham brentfulgham 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!

if (m_responseCompletionHandler)
m_responseCompletionHandler(PolicyAction::Use);
if (!m_responseCompletionHandlers.isEmpty())
m_responseCompletionHandlers.takeFirst()(PolicyAction::Use);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do wonder if we will ever need to move to a model where we get a specific completion handler to match with a specific response. But I think this is correct (and certainly an improvement over existing).

@cdumez cdumez added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 13, 2026
…hree-part.py is a flaky crash

https://bugs.webkit.org/show_bug.cgi?id=314243
rdar://169360779

Reviewed by Brent Fulgham and Brady Eidson.

http/tests/security/contentSecurityPolicy/multipart-three-part.py is
flaky because NetworkResourceLoader::didReceiveResponse() sometimes
overwrites m_responseCompletionHandler before the previous one has been
called, hitting the CompletionHandler destructor assertion.

For a multipart/x-mixed-replace main resource, the network layer can
deliver follow-up parts via didReceiveResponse() before the WebProcess
has answered ContinueDidReceiveResponse for the first part. The first
part's round-trip is async because it goes through
DocumentLoader::responseReceived() -> checkContentPolicy(), which IPCs
to the UIProcess. Assigning the new completion handler over the still-
pending one destroyed it without invoking it, asserting in
CompletionHandler's destructor.

Replace the single m_responseCompletionHandler with a Deque so each
incoming response queues its own handler, and each ContinueDidReceive-
Response IPC drains the front of the queue. This mirrors the WebProcess
flow (it processes each part in order, sending one ContinueDidReceive-
Response per approved part), so per-part validation is preserved. On
loader teardown / convertToDownload, any remaining handlers are drained
with PolicyAction::Ignore.

No new tests, covered by existing test.

* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::~NetworkResourceLoader):
(WebKit::NetworkResourceLoader::convertToDownload):
(WebKit::NetworkResourceLoader::transferToNewWebProcess):
(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::continueDidReceiveResponse):
* Source/WebKit/NetworkProcess/NetworkResourceLoader.h:

Canonical link: https://commits.webkit.org/313118@main
@webkit-commit-queue webkit-commit-queue force-pushed the 314243_multipart_responses branch from 67dff1a to 071edf4 Compare May 13, 2026 01:53
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 313118@main (071edf4): https://commits.webkit.org/313118@main

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

@webkit-commit-queue webkit-commit-queue merged commit 071edf4 into WebKit:main May 13, 2026
@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 May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants