Crash under StringImpl::~StringImpl() in IDBServer::computeSpaceUsedForOrigin(): Thread[5] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x6f697463656e6e6f -> 0x00000063656e6e6f (possible pointer authentication failure)) [ 0] 0x0000000195c88438 JavaScriptCore`WTF::StringImpl::~StringImpl() [inlined] WTF::StringImpl::deref() at StringImpl.h:1074:29 0x0000000195c88428: ldp x29, x30, [sp, #0x10] 0x0000000195c8842c: ldp x20, x19, [sp], #0x20 0x0000000195c88430: retab 0x0000000195c88434: ldr x0, [x19, #0x18] -> 0x0000000195c88438: ldr w8, [x0] 0x0000000195c8843c: subs w8, w8, #0x2 ; =0x2 0x0000000195c88440: b.eq 0x3d458 ; <+272> [inlined] WTF::StringImpl::~StringImpl() at StringImpl.cpp:150 0x0000000195c88444: str w8, [x0] 0x0000000195c88448: mov x0, x19 [ 0] 0x0000000195c88438 JavaScriptCore`WTF::StringImpl::~StringImpl() + 240 at StringImpl.cpp:145 141 } 142 143 ASSERT(ownership == BufferSubstring); 144 ASSERT(substringBuffer()); -> 145 substringBuffer()->deref(); 146 } 147 148 void StringImpl::destroy(StringImpl* stringImpl) 149 { [ 1] 0x0000000195c8849f JavaScriptCore`WTF::StringImpl::destroy(WTF::StringImpl*) [inlined] WTF::StringImpl::~StringImpl() + 3 at StringImpl.cpp:108:1 104 105 StringImpl::StaticStringImpl StringImpl::s_emptyAtomString("", StringImpl::StringAtom); 106 107 StringImpl::~StringImpl() -> 108 { 109 ASSERT(!isStatic()); 110 111 StringView::invalidate(*this); 112 [ 1] 0x0000000195c8849c JavaScriptCore`WTF::StringImpl::destroy(WTF::StringImpl*) + 12 at StringImpl.cpp:150 146 } 147 148 void StringImpl::destroy(StringImpl* stringImpl) 149 { -> 150 stringImpl->~StringImpl(); 151 fastFree(stringImpl); 152 } 153 154 Ref<StringImpl> StringImpl::createFromLiteral(const char* characters, unsigned length) [ 2] 0x0000000195c8849f JavaScriptCore`WTF::StringImpl::destroy(WTF::StringImpl*) [inlined] WTF::StringImpl::~StringImpl() + 3 at StringImpl.cpp:108:1 104 105 StringImpl::StaticStringImpl StringImpl::s_emptyAtomString("", StringImpl::StringAtom); 106 107 StringImpl::~StringImpl() -> 108 { 109 ASSERT(!isStatic()); 110 111 StringView::invalidate(*this); 112 [ 2] 0x0000000195c8849c JavaScriptCore`WTF::StringImpl::destroy(WTF::StringImpl*) + 12 at StringImpl.cpp:150 146 } 147 148 void StringImpl::destroy(StringImpl* stringImpl) 149 { -> 150 stringImpl->~StringImpl(); 151 fastFree(stringImpl); 152 } 153 154 Ref<StringImpl> StringImpl::createFromLiteral(const char* characters, unsigned length) [ 3] 0x0000000195c68703 JavaScriptCore`WTF::FileSystemImpl::pathByAppendingComponent(WTF::String const&, WTF::String const&) + 563 at FileSystemPOSIX.cpp:0 1 /* 2 * Copyright (C) 2007-2017 Apple Inc. All rights reserved. 3 * 4 * Redistribution and use in source and binary forms, with or without 5 * modification, are permitted provided that the following conditions 6 * are met: 7 * 8 * 1. Redistributions of source code must retain the above copyright 9 * notice, this list of conditions and the following disclaimer. ok [ 4] 0x000000018f3c9b77 WebCore`WebCore::IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(WebCore::SecurityOriginData const&, WebCore::SecurityOriginData const&, WTF::String const&, WTF::String const&) + 51 at IDBDatabaseIdentifier.cpp:65:31 61 } 62 63 String IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(const SecurityOriginData& topLevelOrigin, const SecurityOriginData& openingOrigin, const String& rootDirectory, const String& versionString) 64 { -> 65 String versionDirectory = FileSystem::pathByAppendingComponent(rootDirectory, versionString); 66 String mainFrameDirectory = FileSystem::pathByAppendingComponent(versionDirectory, topLevelOrigin.databaseIdentifier()); 67 68 // If the opening origin and main frame origins are the same, there is no partitioning. 69 if (openingOrigin == topLevelOrigin) [ 5] 0x000000018f40a687 WebCore`WebCore::IDBServer::IDBServer::computeSpaceUsedForOrigin(WebCore::ClientOrigin const&) + 79 at IDBServer.cpp:778:38 774 void IDBServer::computeSpaceUsedForOrigin(const ClientOrigin& origin) 775 { 776 ASSERT(!isMainThread()); 777 -> 778 auto oldVersionOriginDirectory = IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(origin.topOrigin, origin.clientOrigin, m_databaseDirectoryPath, "v0"); 779 auto newVersionOriginDirectory = IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(origin.topOrigin, origin.clientOrigin, m_databaseDirectoryPath, "v1"); 780 auto size = SQLiteIDBBackingStore::databasesSizeForFolder(oldVersionOriginDirectory) + SQLiteIDBBackingStore::databasesSizeForFolder(newVersionOriginDirectory); 781 782 postDatabaseTaskReply(createCrossThreadTask(*this, &IDBServer::finishComputingSpaceUsedForOrigin, origin, size)); [ 6] 0x000000018f4172db WebCore`WTF::Detail::CallableWrapper<WTF::CrossThreadTask WTF::createCrossThreadTask<WebCore::IDBServer::IDBServer, 0, WebCore::ClientOrigin const&, WebCore::ClientOrigin>(WebCore::IDBServer::IDBServer&, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), WebCore::ClientOrigin const&)::'lambda'(), void>::call() [inlined] void WTF::callMemberFunctionForCrossThreadTaskImpl<WebCore::IDBServer::IDBServer, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), std::__1::tuple<WebCore::ClientOrigin>, 0ul>(WebCore::IDBServer::IDBServer*, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), std::__1::tuple<WebCore::ClientOrigin>&&, std::__1::integer_sequence<unsigned long, 0ul>) + 63 at CrossThreadTask.h:78:5 [ 6] 0x000000018f41729c WebCore`WTF::Detail::CallableWrapper<WTF::CrossThreadTask WTF::createCrossThreadTask<WebCore::IDBServer::IDBServer, 0, WebCore::ClientOrigin const&, WebCore::ClientOrigin>(WebCore::IDBServer::IDBServer&, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), WebCore::ClientOrigin const&)::'lambda'(), void>::call() [inlined] void WTF::callMemberFunctionForCrossThreadTask<WebCore::IDBServer::IDBServer, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), std::__1::tuple<WebCore::ClientOrigin>, std::__1::integer_sequence<unsigned long, 0ul> >(WebCore::IDBServer::IDBServer*, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), std::__1::tuple<WebCore::ClientOrigin>&&) at CrossThreadTask.h:84 [ 6] 0x000000018f41729c WebCore`WTF::Detail::CallableWrapper<WTF::CrossThreadTask WTF::createCrossThreadTask<WebCore::IDBServer::IDBServer, 0, WebCore::ClientOrigin const&, WebCore::ClientOrigin>(WebCore::IDBServer::IDBServer&, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), WebCore::ClientOrigin const&)::'lambda'(), void>::call() [inlined] WTF::CrossThreadTask WTF::createCrossThreadTask<WebCore::IDBServer::IDBServer, 0, WebCore::ClientOrigin const&, WebCore::ClientOrigin>(WebCore::IDBServer::IDBServer&, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), WebCore::ClientOrigin const&)::'lambda'()::operator()() + 8 at CrossThreadTask.h:99 [ 6] 0x000000018f417294 WebCore`WTF::Detail::CallableWrapper<WTF::CrossThreadTask WTF::createCrossThreadTask<WebCore::IDBServer::IDBServer, 0, WebCore::ClientOrigin const&, WebCore::ClientOrigin>(WebCore::IDBServer::IDBServer&, void (WebCore::IDBServer::IDBServer::*)(WebCore::ClientOrigin const&), WebCore::ClientOrigin const&)::'lambda'(), void>::call() at Function.h:52 [ 7] 0x0000000195c5d9a3 JavaScriptCore`WTF::CrossThreadTaskHandler::taskRunLoop() [inlined] WTF::Function<void ()>::operator()() const + 19 at Function.h:79:35 [ 7] 0x0000000195c5d990 JavaScriptCore`WTF::CrossThreadTaskHandler::taskRunLoop() [inlined] WTF::CrossThreadTask::performTask() at CrossThreadTask.h:48 [ 7] 0x0000000195c5d990 JavaScriptCore`WTF::CrossThreadTaskHandler::taskRunLoop() + 236 at CrossThreadTaskHandler.cpp:78
<rdar://problem/54565546>
Created attachment 376901 [details] Patch
Comment on attachment 376901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376901&action=review > Source/WebCore/Modules/indexeddb/server/IDBServer.h:133 > + String databaseDirectoryPath() const { return m_databaseDirectoryPath.isolatedCopy(); } There's nothing obvious about this code location that indicates it's going to be sent to another thread.
(In reply to Alex Christensen from comment #3) > Comment on attachment 376901 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376901&action=review > > > Source/WebCore/Modules/indexeddb/server/IDBServer.h:133 > > + String databaseDirectoryPath() const { return m_databaseDirectoryPath.isolatedCopy(); } > > There's nothing obvious about this code location that indicates it's going > to be sent to another thread. What's the concern here? Performance? I doubt this is hot. Also, this is private and mostly called from background threads.
My concern is, "are there other strings that are being passed to other threads with this string?" is a hard question to answer by looking at this change.
(In reply to Chris Dumez from comment #4) > (In reply to Alex Christensen from comment #3) > > Comment on attachment 376901 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=376901&action=review > > > > > Source/WebCore/Modules/indexeddb/server/IDBServer.h:133 > > > + String databaseDirectoryPath() const { return m_databaseDirectoryPath.isolatedCopy(); } > > > > There's nothing obvious about this code location that indicates it's going > > to be sent to another thread. > > What's the concern here? Performance? I doubt this is hot. Also, this is > private and mostly called from background threads. Also, we already use this pattern in network cache code at least: String Storage::basePath() const { return m_basePath.isolatedCopy(); } String Storage::versionPath() const { return makeVersionedDirectoryPath(basePath()); } String Storage::recordsPath() const { return m_recordsPath.isolatedCopy(); }
(In reply to Alex Christensen from comment #5) > My concern is, "are there other strings that are being passed to other > threads with this string?" is a hard question to answer by looking at this > change. You can see in the bug description the crash trace: auto oldVersionOriginDirectory = IDBDatabaseIdentifier::databaseDirectoryRelativeToRoot(origin.topOrigin, origin.clientOrigin, m_databaseDirectoryPath, "v0"); and then: String versionDirectory = FileSystem::pathByAppendingComponent(rootDirectory, versionString); So issue is either with m_databaseDirectoryPath or "v0". It cannot be "v0" so it must be m_databaseDirectoryPath. origin is properly isolated copied: postDatabaseTask(createCrossThreadTask(*this, &IDBServer::computeSpaceUsedForOrigin, origin));
Comment on attachment 376901 [details] Patch Clearing flags on attachment: 376901 Committed r248969: <https://trac.webkit.org/changeset/248969>
All reviewed patches have been landed. Closing bug.