We are seeing crashes: Thread 1 name: Dispatch queue: HTTPS Upgrade Checker Thread Thread 1 Crashed ↩: 0 libsqlite3.dylib 0x1ba202ac sqlite3MutexMisuseAssert + 136 1 libsqlite3.dylib 0x1ba202a0 sqlite3MutexMisuseAssert + 124 2 libsqlite3.dylib 0x1ba200dc checkMutexEnter + 56 3 libsqlite3.dylib 0x1b97469c sqlite3_overload_function + 140 4 libsqlite3.dylib 0x1b96e288 openDatabase + 3288 5 WebCore 0x23f8d69c WebCore::SQLiteDatabase::open(WTF::String const&, WebCore::SQLiteDatabase::OpenMode) + 344 (SQLiteDatabase.cpp:116) 6 WebKit 0x02ebae70 WTF::Detail::CallableWrapper<WebKit::NetworkHTTPSUpgradeChecker::NetworkHTTPSUpgradeChecker()::$_18, void>::call() + 80 (NetworkHTTPSUpgradeChecker.cpp:70) 7 libdispatch.dylib 0x1b02d7c0 _dispatch_call_block_and_release + 24 (init.c:1408) 8 libdispatch.dylib 0x1b02eb50 _dispatch_client_callout + 16 (object.m:495)
<rdar://problem/50795714>
Created attachment 371210 [details] Patch
Comment on attachment 371210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371210&action=review > Source/WebKit/ChangeLog:8 > + Can you please explain the "fix" in the change log?
Comment on attachment 371210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371210&action=review > Source/WebKit/ChangeLog:5 > + <rdar://problem/50795714> Should this be a sub-radar of this one? What about all the IDB crashes?
Comment on attachment 371210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371210&action=review Are we sure the issue is not that we're using a background queue as opposed to a background *thread*? Meaning that we do use multiple threads but only 1 thread at a time given that the queue is serial? > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:-58 > - , m_database(makeUniqueRef<WebCore::SQLiteDatabase>()) I do not understand why we need to construct this on the background queue. The SQLiteDatabase constructor does not call any of the sqlite API as far as I know. > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:-59 > - , m_statement(makeUniqueRef<WebCore::SQLiteStatement>(m_database.get(), "SELECT host FROM hosts WHERE host = ?;"_s)) ditto.
Comment on attachment 371210 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371210&action=review > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:70 > + m_database = std::make_unique<SQLiteDatabase>(); Also, now these get constructed on the background queue but still destroyed on the main thread, which may not be safe.
Comment on attachment 371210 [details] Patch Note that I am not opposed to making such a change, especially if this is how our usual database code does it. However: 1. I do not understand why this would fix the crash in question 2. These objects should get destroyed on the background queue if they get constructed on the background queue to avoid introducing new thread-safety bugs
(In reply to Chris Dumez from comment #7) > Comment on attachment 371210 [details] > Patch > > Note that I am not opposed to making such a change, especially if this is > how our usual database code does it. However: > 1. I do not understand why this would fix the crash in question > 2. These objects should get destroyed on the background queue if they get > constructed on the background queue to avoid introducing new thread-safety > bugs 1. The bug should already be fixed as discussed in email. I will change the changelog. 2. Correct. I saw the comment "// This object should be owned by a singleton object." so I didn't destroy the objects. NetworkProcess is not a singleton now, we should add this.
Created attachment 371300 [details] Patch
Comment on attachment 371300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371300&action=review > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:42 > +constexpr auto HTTPSUpgradeCheckerQuery = "SELECT host FROM hosts WHERE host = ?"_s; nit: httpsUpgradeCheckerQuery per WebKit coding style.
Created attachment 371326 [details] Patch for landing
Comment on attachment 371326 [details] Patch for landing Rejecting attachment 371326 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 5000 characters of output: CURSOR_SCALE -DENABLE_NETWORK_CACHE_SPECULATIVE_REVALIDATION -DENABLE_NOTIFICATIONS -DENABLE_PAYMENT_REQUEST -DENABLE_PDFKIT_PLUGIN -DENABLE_POINTER_EVENTS -DENABLE_POINTER_LOCK -DENABLE_PUBLIC_SUFFIX_LIST -DENABLE_REMOTE_INSPECTOR -DENABLE_RESIZE_OBSERVER -DENABLE_RESOURCE_LOAD_STATISTICS -DENABLE_RESOURCE_USAGE -DENABLE_RUBBER_BANDING -DENABLE_SANDBOX_EXTENSIONS -DENABLE_SERVER_PRECONNECT -DENABLE_SERVICE_CONTROLS -DENABLE_SERVICE_WORKER -DENABLE_SHAREABLE_RESOURCE -DENABLE_SPEECH_SYNTHESIS -DENABLE_STREAMS_API -DENABLE_SVG_FONTS -DENABLE_TELEPHONE_NUMBER_DETECTION -DENABLE_TEXT_AUTOSIZING -DENABLE_USERSELECT_ALL -DENABLE_USER_MESSAGE_HANDLERS -DENABLE_VARIATION_FONTS -DENABLE_VIDEO -DENABLE_VIDEO_PRESENTATION_MODE -DENABLE_VIDEO_TRACK -DENABLE_VIDEO_USES_ELEMENT_FULLSCREEN -DENABLE_WEBDRIVER_MOUSE_INTERACTIONS -DENABLE_WEBDRIVER_KEYBOARD_INTERACTIONS -DENABLE_WEBGL -DENABLE_WEBGL2 -DENABLE_WEBGPU -DENABLE_WEB_AUDIO -DENABLE_WEB_AUTHN -DENABLE_WEB_CRYPTO -DENABLE_WEB_PROCESS_SANDBOX -DENABLE_WEB_RTC -DENABLE_WIRELESS_PLAYBACK_TARGET -DENABLE_XSLT -DENABLE_MANUAL_SANDBOXING -DHAVE_CORE_PREDICTION -DU_HIDE_DEPRECATED_API -DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0 -DFRAMEWORK_NAME=WebKit -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -fasm-blocks -fstrict-aliasing -Wprotocol -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.13 -g -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -Wno-sign-conversion -Winfinite-recursion -Wmove -Wcomma -Wblock-capture-autoreleasing -Wstrict-prototypes -Wrange-loop-analysis -Wno-semicolon-before-method-body -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-generated-files.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/ForwardingHeaders -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2 -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/WebKitAdditions -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/local/include/WebKitAdditions -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/webrtc -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/local/include/webrtc -I/Volumes/Data/EWS/WebKit/Source/WebKit -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wextra-tokens -Wformat-security -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wno-unused-parameter -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/PrivateFrameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/PrivateFrameworks -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/System.framework/PrivateHeaders -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/WebKit2Prefix-gljelmzeecuvvkhaymcxhnhphybs/WebKit2Prefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource20-mm.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource20-mm.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource20-mm.mm -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource20-mm.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource3.o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource3.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: https://webkit-queues.webkit.org/results/12376773
Created attachment 371513 [details] Patch for landing
Comment on attachment 371513 [details] Patch for landing Clearing flags on attachment: 371513 Committed r246163: <https://trac.webkit.org/changeset/246163>
All reviewed patches have been landed. Closing bug.