283053 2024-11-13 07:49:52 -0800 [GLIB] Invalid write reported by Valgrind on GSocketMonitor::sourceSocketCallback 2024-11-14 09:28:34 -0800 1 1 1 Unclassified WebKit WPE WebKit WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 lmoura lmoura bugs-noreply mcatanzaro oldest_to_newest 2074820 0 lmoura 2024-11-13 07:49:52 -0800 bug282910 originally reported an invalid memory access in GSocketMonitor::sourceSocketCallback. Originally I thought it was related to the SessionHost destructor being called prematurely, but even after ensuring we'd delete the SessionHost after it returns from the socket message handlers in https://github.com/WebKit/WebKit/pull/36533, the error continues. From the trace, somehow erasing `monitor->m_callback` (added in 285339@main) is triggering the destruction of the monitor immediately while still inside its socketSourceCallback. Does not seem to bring issues in release builds, but running under valgrind shows an invalid write while updating m_shouldDestroyCallback Maybe it's a matter of ensuring cleaning the callback is the last action in the sourceSocket callback. ``` ** (WPEWebProcess:2182124): WARNING **: 23:33:30.451: RemoteInspector failed to connect to inspector server at: 127.0.0.1:40907: GDBus.Error:org.freedesktop.portal.Error.NotAllowed: This call is not available inside the sandbox ==2181500== Invalid write of size 1 ==2181500== at 0x451DC8: WTF::GSocketMonitor::socketSourceCallback(_GSocket*, GIOCondition, WTF::GSocketMonitor*) (GSocketMonitor.cpp:51) ==2181500== by 0x4BCDBB0: socket_source_dispatch (gsocket.c:4267) ==2181500== by 0x4DC148D: g_main_dispatch.lto_priv.0 (gmain.c:3344) ==2181500== by 0x4E20716: UnknownInlinedFun (gmain.c:4152) ==2181500== by 0x4E20716: g_main_context_iterate_unlocked.isra.0 (gmain.c:4217) ==2181500== by 0x4DC1F76: g_main_loop_run (gmain.c:4419) ==2181500== by 0x453161: WTF::RunLoop::run() (RunLoopGLib.cpp:108) ==2181500== by 0x3781F5: WebDriver::WebDriverService::run(int, char**) (WebDriverService.cpp:186) ==2181500== by 0x375AED: main (WebDriverMain.cpp:39) ==2181500== Address 0x7aa7619 is 73 bytes inside a block of size 128 free'd ==2181500== at 0x484988F: free (vg_replace_malloc.c:985) ==2181500== by 0x45E459: bmalloc::DebugHeap::free(void*) (DebugHeap.cpp:139) ==2181500== by 0x45E739: pas_debug_heap_free (DebugHeap.cpp:238) ==2181500== by 0x4BAB8C: pas_try_deallocate_slow_no_cache (pas_deallocate.c:135) ==2181500== by 0x3E0BEC: pas_try_deallocate (pas_deallocate.h:209) ==2181500== by 0x3E0C75: pas_deallocate (pas_deallocate.h:221) ==2181500== by 0x3E579F: bmalloc_deallocate_inline (bmalloc_heap_inlines.h:592) ==2181500== by 0x3E5EFC: free (bmalloc.h:187) ==2181500== by 0x3E5EFC: WTF::fastFree(void*) (FastMalloc.cpp:617) ==2181500== by 0x3BD846: WTF::RefCounted<WTF::SocketConnection>::operator delete(void*) (RefCounted.h:199) ==2181500== by 0x3BC218: WTF::RefCounted<WTF::SocketConnection>::deref() const (RefCounted.h:204) ==2181500== by 0x3BA9D1: WTF::DefaultRefDerefTraits<WTF::SocketConnection>::derefIfNotNull(WTF::SocketConnection*) (Ref.h:62) ==2181500== by 0x3B89BC: WTF::Ref<WTF::SocketConnection, WTF::RawPtrTraits<WTF::SocketConnection>, WTF::DefaultRefDerefTraits<WTF::SocketConnection> >::~Ref() (Ref.h:82) ==2181500== Block was alloc'd at ==2181500== at 0x4846828: malloc (vg_replace_malloc.c:442) ==2181500== by 0x45E370: bmalloc::DebugHeap::malloc(unsigned long, bmalloc::FailureAction) (DebugHeap.cpp:118) ==2181500== by 0x45E5C6: pas_debug_heap_malloc (DebugHeap.cpp:223) ==2181500== by 0x462DF3: pas_debug_heap_allocate (pas_debug_heap.h:106) ==2181500== by 0x46374E: pas_try_allocate_intrinsic_impl_casual_case (pas_try_allocate_intrinsic.h:105) ==2181500== by 0x463F44: bmalloc_allocate_impl_casual_case (bmalloc_heap_inlines.h:69) ==2181500== by 0x464441: bmalloc_allocate_casual (bmalloc_heap.c:64) ==2181500== by 0x3E4A21: bmalloc_allocate_inline (bmalloc_heap_inlines.h:120) ==2181500== by 0x3E5995: malloc (bmalloc.h:79) ==2181500== by 0x3E5995: WTF::fastMalloc(unsigned long) (FastMalloc.cpp:559) ==2181500== by 0x3B892B: WTF::RefCounted<WTF::SocketConnection>::operator new(unsigned long) (RefCounted.h:199) ==2181500== by 0x3B51A7: WTF::SocketConnection::create(WTF::GRefPtr<_GSocketConnection>&&, WTF::HashMap<WTF::CString, std::pair<WTF::CString, void (*)(WTF::SocketConnection&, _GVariant*, void*)>, WTF::DefaultHash<WTF::CString>, WTF::HashTraits<WTF::CString>, WTF::HashTraits<std::pair<WTF::CString, void (*)(WTF::SocketConnection&, _GVariant*, void*)> >, WTF::HashTableTraits, (WTF::ShouldValidateKey)0> const&, void*) (SocketConnection.h:41) ==2181500== by 0x3B6A91: WebDriver::SessionHost::connectToBrowser(std::unique_ptr<WebDriver::ConnectToBrowserAsyncData, std::default_delete<WebDriver::ConnectToBrowserAsyncData> >&&)::{lambda()#1}::operator()()::{lambda(_GObject*, _GAsyncResult*, void*)#1}::operator()(_GObject*, _GAsyncResult*, void*) const (SessionHostGlib.cpp:217) ==2181500== ``` Tentative patch incoming. 2074877 1 mcatanzaro 2024-11-13 11:43:52 -0800 (In reply to Lauro Moura from comment #0) > Tentative patch incoming. Thanks :) 2075043 2 lmoura 2024-11-14 06:00:54 -0800 Pull request: https://github.com/WebKit/WebKit/pull/36635 2075101 3 ews-feeder 2024-11-14 09:28:32 -0800 Committed 286596@main (5d047d3c9925): <https://commits.webkit.org/286596@main> Reviewed commits have been landed. Closing PR #36635 and removing active labels.