Bug 236209 – REGRESSION(r286936): Crash in WebKit::OriginStorageManager::StorageBucket::deleteLocalStorageData

• Component: Website Storage
• Status: RESOLVED FIXED

[Michael Catanzaro]

Created attachment 451052 [details]
Full backtrace

It seems that the network process is fairly crashy in WebKitGTK 2.35.2. This seems to be a regression from 2.35.1, probably related to recent storage refactoring:

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1  0x00007fecbeea5867 in __GI_abort () at abort.c:79
#2  0x00007fecbf71f5c6 in std::__replacement_assert(char const*, int, char const*, char const*)
    (__file=__file@entry=0x7fecc1ed58b0 "/usr/include/c++/11.2.0/optional", __line=__line@entry=440, __function=__function@entry=0x7fecc1ee9f10 "constexpr _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() [with _Tp = WTF::WallTime; _Dp = std::_Optional_base<WTF::WallTime, true, true>]", __condition=__condition@entry=0x7fecc1ed504c "this->_M_is_engaged()") at /usr/include/c++/11.2.0/x86_64-unknown-linux-gnu/bits/c++config.h:514
#3  0x00007fecbfa597af in std::_Optional_base_impl<WTF::WallTime, std::_Optional_base<WTF::WallTime, true, true> >::_M_get() (this=0x7fec57bfe6f0) at /usr/include/c++/11.2.0/optional:438
#4  std::_Optional_base_impl<WTF::WallTime, std::_Optional_base<WTF::WallTime, true, true> >::_M_get()
    (this=0x7fec57bfe6f0) at /usr/include/c++/11.2.0/optional:438
#5  std::optional<WTF::WallTime>::operator*() & (this=0x7fec57bfe6f0) at /usr/include/c++/11.2.0/optional:927
#6  WebKit::OriginStorageManager::StorageBucket::deleteLocalStorageData(WTF::WallTime) (time=..., this=0x7fec57ceaaf0)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp:249
#7  WebKit::OriginStorageManager::StorageBucket::deleteData(WTF::OptionSet<WebKit::WebsiteDataType>, WTF::WallTime)
    (modifiedSinceTime=..., types=..., this=0x7fec57ceaaf0)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp:185
#8  WebKit::OriginStorageManager::deleteData(WTF::OptionSet<WebKit::WebsiteDataType>, WTF::WallTime) (
    this=<optimized out>, types=..., modifiedSince=...)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp:364
#9  0x00007fecbfa5aa3f in WebKit::NetworkStorageManager::deleteDataOnDisk(WTF::OptionSet<WebKit::WebsiteDataType>, WTF::WallTime, WTF::Function<bool (WebCore::ClientOrigin const&)> const&)
    (this=this@entry=0x7fecb7cde3a8, types=..., modifiedSinceTime=..., modifiedSinceTime@entry=..., filter=...)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp:500
#10 0x00007fecbfa5e156 in operator() (__closure=<optimized out>) at /usr/include/c++/11.2.0/bits/unique_ptr.h:172
#11 WTF::Detail::CallableWrapper<WebKit::NetworkStorageManager::deleteDataForRegistrableDomains(WTF::OptionSet<WebKit::WebsiteDataType>, const WTF::Vector<WebCore::RegistrableDomain>&, WTF::CompletionHandler<void(WTF::HashSet<WebCore::RegistrableDomain>&&)>&&)::<lambda()>, void>::call(void) (this=0x7febf4023c08)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/_builddir/WTF/Headers/wtf/Function.h:53
#12 0x00007fecbe63afdd in WTF::Function<void ()>::operator()() const (this=<synthetic pointer>)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/Function.h:79
#13 WTF::RunLoop::performWork() (this=0x7fecb7c6b000)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/RunLoop.cpp:133
#14 0x00007fecbe69b1bd in operator() (userData=<optimized out>, __closure=0x0)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/glib/RunLoopGLib.cpp:80
#15 _FUN(gpointer) () at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/glib/RunLoopGLib.cpp:82
#16 0x00007fecbe69bbad in operator()
    (__closure=0x0, userData=0x7fecb7c6b000, callback=0x7fecbe69b1b0 <_FUN(gpointer)>, source=0x7fec50003e80)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/glib/RunLoopGLib.cpp:53
#17 _FUN(GSource*, GSourceFunc, gpointer) ()
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/glib/RunLoopGLib.cpp:56
#18 0x00007fecbea68c6b in g_main_dispatch (context=0x7fec50000b60) at ../glib/gmain.c:3413
#19 g_main_context_dispatch (context=0x7fec50000b60) at ../glib/gmain.c:4131
#20 0x00007fecbea69178 in g_main_context_iterate
    (context=0x7fec50000b60, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
    at ../glib/gmain.c:4207
#21 0x00007fecbea69493 in g_main_loop_run (loop=0x7fec50003e60) at ../glib/gmain.c:4405
#22 0x00007fecbe69bd10 in WTF::RunLoop::run() ()
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/glib/RunLoopGLib.cpp:108
#23 0x00007fecbe63d8c5 in WTF::Function<void ()>::operator()() const (this=<synthetic pointer>)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/Function.h:79
#24 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (newThreadContext=0x7fecb7cf0120)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/Threading.cpp:191
#25 0x00007fecbe69e98d in WTF::wtfThreadEntryPoint(void*) (context=<optimized out>)
    at /usr/lib/debug/source/sdk/webkit2gtk-4.0.bst/Source/WTF/wtf/posix/ThreadingPOSIX.cpp:244
#26 0x00007fecbcd7e3ba in start_thread (arg=0x7fec57bff640) at pthread_create.c:481
#27 0x00007fecbef84b03 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

I'll attach a full backtrace.

[Sihui Liu]

Created attachment 451055 [details]
Patch

[Darin Adler]

Comment on attachment 451055 [details]
Patch

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

> Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp:250
> -        if (auto modificationTime = FileSystem::fileModificationTime(m_localStoragePath); *modificationTime >= time) {
> +        auto modificationTime = FileSystem::fileModificationTime(m_localStoragePath);
> +        if (modificationTime && *modificationTime >= time) {

Since C++17, std::optional has a more economical way to write this. You can just remove the "*". Also, no need for a local variable:

    if (FileSystem::fileModificationTime(m_localStoragePath) >= time) {

The >= expression will evaluate to false if the optional is nullopt.

[Sihui Liu]

(In reply to Darin Adler from comment #2)
> Comment on attachment 451055 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=451055&action=review
> 
> > Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp:250
> > -        if (auto modificationTime = FileSystem::fileModificationTime(m_localStoragePath); *modificationTime >= time) {
> > +        auto modificationTime = FileSystem::fileModificationTime(m_localStoragePath);
> > +        if (modificationTime && *modificationTime >= time) {
> 
> Since C++17, std::optional has a more economical way to write this. You can
> just remove the "*". Also, no need for a local variable:
> 
>     if (FileSystem::fileModificationTime(m_localStoragePath) >= time) {
> 
> The >= expression will evaluate to false if the optional is nullopt.

Ah, good to know! Will make the change.

[Sihui Liu]

Created attachment 451065 [details]
Patch for landing

[EWS]

Committed r289200 (246886@main): <https://commits.webkit.org/246886@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/88556107>