Bug 187243 – Crash in WebKit::CacheStorage::Cache::toRecordInformation when running http/tests/cache-storage/cache-persistency.https.html

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Frédéric Wang (:fredw)]

Test http/tests/cache-storage/cache-persistency.https.html is crashing on the EWS in bug #186536, where I fix WTF's internal std::optional to crash when dereferencing nullopt (instead of returning 0 as it currently does). We'll likely add a crash expectation, which should be removed when this is fixed.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x000000010833bf6f WebKit::CacheStorage::Caches::salt() const + 61 (Optional.h:552)
1   com.apple.WebKit              	0x0000000108331737 WebKit::CacheStorage::Cache::toRecordInformation(WebCore::DOMCacheEngine::Record const&) + 99 (CacheStorageEngineCache.cpp:100)
2   com.apple.WebKit              	0x0000000108332475 WebKit::CacheStorage::Cache::addRecord(WTF::Vector<WebKit::CacheStorage::RecordInformation, 0ul, WTF::CrashOnOverflow, 16ul>*, WebCore::DOMCacheEngine::Record const&) + 295 (Vector.h:691)
3   com.apple.WebKit              	0x000000010833283d WebKit::CacheStorage::Cache::storeRecords(WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&, WTF::Function<void (std::experimental::fundamentals_v3::expected<WTF::Vector<unsigned long long, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::DOMCacheEngine::Error>&&)>&&) + 533 (Ref.h:65)
4   com.apple.WebKit              	0x0000000108332d98 WebKit::CacheStorage::Cache::put(WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&, WTF::Function<void (std::experimental::fundamentals_v3::expected<WTF::Vector<unsigned long long, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::DOMCacheEngine::Error>&&)>&&) + 700 (CacheStorageEngineCache.cpp:422)
5   com.apple.WebKit              	0x000000010832b18a WebKit::CacheStorage::Engine::readCache(unsigned long long, WTF::Function<void (std::experimental::fundamentals_v3::expected<std::__1::reference_wrapper<WebKit::CacheStorage::Cache>, WebCore::DOMCacheEngine::Error>&&)>&&) + 82 (Function.h:56)
6   com.apple.WebKit              	0x000000010832b275 WebKit::CacheStorage::Engine::putRecords(unsigned long long, WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&, WTF::Function<void (std::experimental::fundamentals_v3::expected<WTF::Vector<unsigned long long, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::DOMCacheEngine::Error>&&)>&&) + 131 (memory:2549)
7   com.apple.WebKit              	0x0000000108329d7b WebKit::CacheStorage::Engine::from(PAL::SessionID, WTF::Function<void (WebKit::CacheStorage::Engine&)>&&) + 161
8   com.apple.WebKit              	0x000000010832a3fd WebKit::CacheStorage::Engine::putRecords(PAL::SessionID, unsigned long long, WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&, WTF::Function<void (std::experimental::fundamentals_v3::expected<WTF::Vector<unsigned long long, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::DOMCacheEngine::Error>&&)>&&) + 149 (memory:2549)
9   com.apple.WebKit              	0x000000010833f742 WebKit::CacheStorageEngineConnection::putRecords(PAL::SessionID, unsigned long long, unsigned long long, WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&) + 246 (memory:2549)
10  com.apple.WebKit              	0x0000000108341e94 void IPC::handleMessage<Messages::CacheStorageEngineConnection::PutRecords, WebKit::CacheStorageEngineConnection, void (WebKit::CacheStorageEngineConnection::*)(PAL::SessionID, unsigned long long, unsigned long long, WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&)>(IPC::Decoder&, WebKit::CacheStorageEngineConnection*, void (WebKit::CacheStorageEngineConnection::*)(PAL::SessionID, unsigned long long, unsigned long long, WTF::Vector<WebCore::DOMCacheEngine::Record, 0ul, WTF::CrashOnOverflow, 16ul>&&)) + 98 (HandleMessage.h:41)
11  com.apple.WebKit              	0x0000000108345cff IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 119 (memory:2521)
12  com.apple.WebKit              	0x0000000108348c8f IPC::Connection::dispatchOneIncomingMessage() + 177 (Connection.cpp:1035)
13  JavaScriptCore                	0x000000011020d5cc WTF::RunLoop::performWork() + 236 (Function.h:56)
14  JavaScriptCore                	0x000000011020d872 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39)
15  com.apple.CoreFoundation      	0x0000000109b4fbb1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
16  com.apple.CoreFoundation      	0x0000000109b344af __CFRunLoopDoSources0 + 271
17  com.apple.CoreFoundation      	0x0000000109b33a6f __CFRunLoopRun + 1263
18  com.apple.CoreFoundation      	0x0000000109b3330b CFRunLoopRunSpecific + 635
19  com.apple.Foundation          	0x0000000107cfbb4a -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 274
20  com.apple.Foundation          	0x0000000107cfba25 -[NSRunLoop(NSRunLoop) run] + 76
21  libxpc.dylib                  	0x000000010b75b8c9 _xpc_objc_main + 460
22  libxpc.dylib                  	0x000000010b75dd73 xpc_main + 143
23  com.apple.WebKit.Networking   	0x0000000107c5144d main + 403 (OSObjectPtr.h:69)
24  libdyld.dylib                 	0x000000010b3a3955 start + 1

[Frédéric Wang (:fredw)]

Same crash sometimes happens with
http/tests/cache-storage/cache-representation.https.html

https://trac.webkit.org/changeset/233418/

[youenn fablet]

Created attachment 344136 [details]
Patch

[Frédéric Wang (:fredw)]

Comment on attachment 344136 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344136&action=review

> LayoutTests/TestExpectations:-390
> -webkit.org/b/187243 http/tests/cache-storage/cache-persistency.https.html [ Crash ]

Can you also please remove the "Crash" for cache-representation.https.html? See comment 1 (I was not sure whether TestExpectations accept multiple bug references...)

[youenn fablet]

Created attachment 344155 [details]
Patch

[EWS Watchlist]

Comment on attachment 344155 [details]
Patch

Attachment 344155 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8419260

New failing tests:
http/tests/security/local-video-source-from-remote.html

[EWS Watchlist]

Created attachment 344160 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[youenn fablet]

Comment on attachment 344155 [details]
Patch

Error is unrelated

[Frédéric Wang (:fredw)]

Comment on attachment 344155 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344155&action=review

LGTM.

It would be great to take this ASAP since it is making execution of WK2 layout tests regularly fail e.g.
https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r233451%20(7099)/results.html
https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233453%20(5961)/results.html

> Source/WebKit/ChangeLog:3
> +        Crash in  WebKit::CacheStorage::Cache::toRecordInformation when running http/tests/cache-storage/cache-persistency.https.html

nit: there is a double space after "Crash in"

> LayoutTests/ChangeLog:3
> +        Crash in  WebKit::CacheStorage::Cache::toRecordInformation when running http/tests/cache-storage/cache-persistency.https.html

Ditto.

[Chris Dumez]

Comment on attachment 344155 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344155&action=review

> Source/WebKit/ChangeLog:7
> +

Please explain the fix in the changelog.

[youenn fablet]

Created attachment 344199 [details]
Patch

[Frédéric Wang (:fredw)]

Comment on attachment 344199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344199&action=review

> LayoutTests/TestExpectations:389
>  webkit.org/b/187145 imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-no-browsing-context.html [ Crash ]

The conflict is probably due to https://trac.webkit.org/changeset/233458/webkit

[youenn fablet]

Created attachment 344244 [details]
Rebasing

[WebKit Commit Bot]

Comment on attachment 344244 [details]
Rebasing

Clearing flags on attachment: 344244

Committed r233492: <https://trac.webkit.org/changeset/233492>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41803068>