Bug 201324 - Remove StorageArea pointers in StorageManagerSet when removing StorageManagers that own them
Summary: Remove StorageArea pointers in StorageManagerSet when removing StorageManager...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-29 18:37 PDT by Alex Christensen
Modified: 2019-09-05 10:12 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.28 KB, patch)
2019-08-29 18:39 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (9.86 KB, patch)
2019-09-04 13:27 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-08-29 18:37:23 PDT
Remove StorageArea pointers in StorageManagerSet when removing StorageManagers that own them
Comment 1 Alex Christensen 2019-08-29 18:39:38 PDT
Created attachment 377666 [details]
Patch

This prevents crashes that look like this:

Thread 7 Crashed:: Dispatch queue: com.apple.WebKit.WebStorage
0   com.apple.WebKit              	0x0000000101cd72e8 WTF::HashTableConstIterator<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType>, WTF::ObjectIdentifier<IPC::Connection::UniqueIDType>, WTF::IdentityExtractor, WTF::ObjectIdentifierHash<IPC::Connection::UniqueIDType>, WTF::HashTraits<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> >, WTF::HashTraits<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> > > WTF::HashTable<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType>, WTF::ObjectIdentifier<IPC::Connection::UniqueIDType>, WTF::IdentityExtractor, WTF::ObjectIdentifierHash<IPC::Connection::UniqueIDType>, WTF::HashTraits<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> >, WTF::HashTraits<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> > >::find<WTF::IdentityHashTranslator<WTF::HashTraits<WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> >, WTF::ObjectIdentifierHash<IPC::Connection::UniqueIDType> >, WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> >(WTF::ObjectIdentifier<IPC::Connection::UniqueIDType> const&) const + 106 (ObjectIdentifier.h:77)
1   com.apple.WebKit              	0x0000000101cc9308 WebKit::StorageArea::removeListener(WTF::ObjectIdentifier<IPC::Connection::UniqueIDType>) + 36 (HashTable.h:383)
2   com.apple.WebKit              	0x0000000101cd8e28 WTF::Detail::CallableWrapper<WebKit::StorageManagerSet::removeConnection(IPC::Connection&)::$_8, void>::call() + 106 (HashTable.h:183)
3   libdispatch.dylib             	0x00007fff70501553 _dispatch_call_block_and_release + 12
4   libdispatch.dylib             	0x00007fff705024de _dispatch_client_callout + 8
5   libdispatch.dylib             	0x00007fff70507a9e _dispatch_lane_serial_drain + 597
6   libdispatch.dylib             	0x00007fff70508422 _dispatch_lane_invoke + 363
7   libdispatch.dylib             	0x00007fff70511aa1 _dispatch_workloop_worker_thread + 598
8   libsystem_pthread.dylib       	0x00007fff7075b763 _pthread_wqthread + 290
9   libsystem_pthread.dylib       	0x00007fff7075b5bf start_wqthread + 15
Comment 2 Sihui Liu 2019-08-29 22:56:10 PDT
Comment on attachment 377666 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        Remove StorageArea pointers in StorageManagerSet when removing StorageManagers that own them

Nice!

> Source/WebKit/NetworkProcess/WebStorage/StorageManagerSet.cpp:86
> +            if (auto manager = m_storageManagers.get(sessionID)) {
> +                for (auto& localStorageNamespace : manager->localStorageNamespaces()) {
> +                    for (auto& storageArea : localStorageNamespace->storageAreas())
> +                        m_storageAreas.remove(storageArea->identifier());
> +                }
> +            }

We probably also need to clear StorageAreas from m_transientLocalStorageNamespaces and m_sessionStorageNamespaces of StorageManager. 

Maybe just let StorageManager to return allStorageAreaIdentifiers()?
Comment 3 Sihui Liu 2019-09-04 13:27:40 PDT
Created attachment 378008 [details]
Patch
Comment 4 WebKit Commit Bot 2019-09-05 10:12:00 PDT
Comment on attachment 378008 [details]
Patch

Clearing flags on attachment: 378008

Committed r249533: <https://trac.webkit.org/changeset/249533>
Comment 5 WebKit Commit Bot 2019-09-05 10:12:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2019-09-05 10:12:18 PDT
<rdar://problem/55072253>