Bug 225522 – [ macOS Release wk2 ] http/tests/cache-storage/cache-records-persistency.https.html is flaky crashing

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

[Robert Jenner]

http/tests/cache-storage/cache-records-persistency.https.html

is a flaky crash on macOS Catalina and BigSur Release wk2. 

HISTORY:
https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2Fcache-storage%2Fcache-records-persistency.https.html

CRASH TEXT:
Thread 2 Crashed:: Dispatch queue: com.apple.WebKit.Cache.Storage.background
0   libsystem_kernel.dylib        	0x00007fff2037c946 __pthread_kill + 10
1   libsystem_pthread.dylib       	0x00007fff203ab615 pthread_kill + 263
2   libsystem_c.dylib             	0x00007fff20300411 abort + 120
3   libc++abi.dylib               	0x00007fff2036eef2 abort_message + 241
4   libc++abi.dylib               	0x00007fff203605e5 demangling_terminate_handler() + 242
5   libobjc.A.dylib               	0x00007fff20259595 _objc_terminate() + 104
6   libc++abi.dylib               	0x00007fff2036e307 std::__terminate(void (*)()) + 8
7   libc++abi.dylib               	0x00007fff2036e2a9 std::terminate() + 41
8   libdispatch.dylib             	0x00007fff202017fa _dispatch_client_callout + 28
9   libdispatch.dylib             	0x00007fff20204190 _dispatch_continuation_pop + 423
10  libdispatch.dylib             	0x00007fff20203915 _dispatch_async_redirect_invoke + 882
11  libdispatch.dylib             	0x00007fff202107f8 _dispatch_root_queue_drain + 326
12  libdispatch.dylib             	0x00007fff20210f50 _dispatch_worker_thread2 + 92
13  libsystem_pthread.dylib       	0x00007fff203a847a _pthread_wqthread + 244
14  libsystem_pthread.dylib       	0x00007fff203a7493 start_wqthread + 15

https://build.webkit.org/results/Apple-BigSur-Release-WK2-Tests/r277174%20(2405)/http/tests/cache-storage/cache-records-persistency.https-crash-log.txt

[Robert Jenner]

Created attachment 428011 [details]
Crashlog for test

Attaching full crashlog to bug.

[Robert Jenner]

I was able to reproduce the crash at BigSur Release ToT using the following test:

run-webkit-tests http/tests/cache-storage/cache-records-persistency.https.html --iterations 1000 -f

I am working on narrowing the regression point. So far I have found that the issue was introduced somewhere in between r276902 and r276971. I will continue narrowing that down.

[Radar WebKit Bug Importer]

<rdar://problem/77680019>

[Robert Jenner]

I have updated the test expectations from Pass Failure, to Pass Failure Crash here:
https://trac.webkit.org/changeset/277298/webkit

Still working on verifying a regression point. It now appears that it is somewhere between r276902 and r276911.

[Robert Jenner]

The regression range is invalid, as I was able to reproduce the crashing test even further back, then I initially thought. At this time, I can't figure out what the regression point of this test was.

[Alexey Proskuryakov]

terminating with uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in directory_iterator::directory_iterator(...): No such file or directory [/var/folders/nb/x54nnkxj3qs9r19fsps6cj880000gn/T/WebKitTestRunners-gCzZxC/CacheStorage/2915450984/2B0047B0E18033807A0A4BD1163F2D17595AB5A2/Development/Version 16/Records/B69DB1D16DEA04A8B3F94C84E35198A088A031DD]

[Alex Christensen]

This looks related to Chris's recent std::filesystem fun

[Chris Dumez]

(In reply to Alex Christensen from comment #7)
> This looks related to Chris's recent std::filesystem fun

Ok. Looking now.

[Chris Dumez]

Will have a fix shortly.

[Chris Dumez]

Created attachment 428232 [details]
Patch

[Chris Dumez]

Regression from https://commits.webkit.org/r276906.

[Darin Adler]

Comment on attachment 428232 [details]
Patch

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

> Source/WTF/ChangeLog:9
> +        Pass ec parameter to std::filesystem::directory_iterator() so that it doesn't throw in case of the

Did you search for other calls that forgot "ec"? Obviously if we accidentally call any of the exception-throwing variants that is not good.

[Chris Dumez]

(In reply to Darin Adler from comment #12)
> Comment on attachment 428232 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=428232&action=review
> 
> > Source/WTF/ChangeLog:9
> > +        Pass ec parameter to std::filesystem::directory_iterator() so that it doesn't throw in case of the
> 
> Did you search for other calls that forgot "ec"? Obviously if we
> accidentally call any of the exception-throwing variants that is not good.

I'll double check. I missed this one because I did not expect a constructor to take an ec.

[Darin Adler]

I found these:

Tools/TestRunnerShared/TestCommand.cpp:    return std::filesystem::absolute(pathOrURL);
Tools/TestRunnerShared/TestCommand.cpp:    return "file://" + std::filesystem::absolute(pathOrURL).generic_string();
Tools/TestRunnerShared/TestFeatures.cpp:    if (!std::filesystem::exists(path))

All in the test runner, though.

[Chris Dumez]

(In reply to Darin Adler from comment #14)
> I found these:
> 
> Tools/TestRunnerShared/TestCommand.cpp:    return
> std::filesystem::absolute(pathOrURL);
> Tools/TestRunnerShared/TestCommand.cpp:    return "file://" +
> std::filesystem::absolute(pathOrURL).generic_string();
> Tools/TestRunnerShared/TestFeatures.cpp:    if
> (!std::filesystem::exists(path))
> 
> All in the test runner, though.

I have just checked in WTF::FileSystem and I did not find any missed ec.

I did not realize our TestRunner was using std::filesystem directly. That's not a recent change from me but I'll fix it as needed.

[EWS]

Committed r277315 (237574@main): <https://commits.webkit.org/237574@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428232 [details].