From 55361bf402090388f0d4800d9289bf635c34e5b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 24 Aug 2020 17:06:58 +0000 Subject: [PATCH] Bug 1660660 - Fix deadlock in gamepad initialization code. r=cmartin StartGamepadMonitoring() can end up in AddGamepad, and acquire the lock again on the same thread, effectively dead-locking. This is a regression from bug 1657404. Relevant stack: (gdb) bt #0 0x00007fd19bace801 in clock_nanosleep@GLIBC_2.2.5 () at /lib64/libc.so.6 #1 0x00007fd19bad4157 in nanosleep () at /lib64/libc.so.6 #2 0x00007fd19bad408e in sleep () at /lib64/libc.so.6 #3 0x00007fd195233e87 in ah_crap_handler(int) (signum=11) at /home/emilio/src/moz/gecko-4/toolkit/xre/nsSigHandlers.cpp:95 #4 0x00007fd1952165c4 in nsProfileLock::FatalSignalHandler(int, siginfo_t*, void*) (signo=11, info=0x7fd14abb9db0, context=0x7fd14abb9c80) at /home/emilio/src/moz/gecko-4/toolkit/profile/nsProfileLock.cpp:177 #5 0x00007fd1964973b2 in WasmTrapHandler(int, siginfo_t*, void*) (signum=11, info=, context=) at /home/emilio/src/moz/gecko-4/js/src/wasm/WasmSignalHandlers.cpp:978 #6 0x00007fd19bf3ca90 in () at /lib64/libpthread.so.0 #7 mozilla::detail::MutexImpl::mutexLock() (this=) at /home/emilio/src/moz/gecko-4/mozglue/misc/Mutex_posix.cpp:118 #8 mozilla::detail::MutexImpl::lock() (this=) at /home/emilio/src/moz/gecko-4/mozglue/misc/Mutex_posix.cpp:142 #9 0x00007fd190cc795a in mozilla::OffTheBooksMutex::Lock() (this=0x7fd136649398) at /home/emilio/src/moz/gecko-4/xpcom/threads/BlockingResourceBase.cpp:318 #10 0x00007fd19326e65e in mozilla::detail::BaseAutoLock::BaseAutoLock(mozilla::Mutex&) (this=, aLock=...) at /home/emilio/src/moz/gecko-4/obj-debug-no-sccache/dist/include/mozilla/Mutex.h:159 #11 mozilla::dom::GamepadPlatformService::NotifyGamepadChange(unsigned int, mozilla::dom::GamepadAdded const&) (this=0x7fd136649380, aIndex=1, aInfo=...) at /home/emilio/src/moz/gecko-4/dom/gamepad/GamepadPlatformService.cpp:65 #12 0x00007fd193269178 in mozilla::dom::GamepadPlatformService::AddGamepad(char const*, mozilla::dom::GamepadMappingType, mozilla::dom::GamepadHand, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int) (this=0x7fd136649380, aID=, aMapping=mozilla::dom::GamepadMappingType::_empty, aHand=mozilla::dom::GamepadHand::_empty, aNumButtons=11, aNumAxes=8, aHaptics=0, aNumLightIndicator=0, aNumTouchEvents=0) at /home/emilio/src/moz/gecko-4/dom/gamepad/GamepadPlatformService.cpp:96 #13 0x00007fd19326de4a in (anonymous namespace)::LinuxGamepadService::AddDevice(mozilla::udev_device*) (this=, dev=) at /home/emilio/src/moz/gecko-4/dom/gamepad/linux/LinuxGamepad.cpp:139 #14 0x00007fd19326a156 in (anonymous namespace)::LinuxGamepadService::ScanForDevices() (this=) at /home/emilio/src/moz/gecko-4/dom/gamepad/linux/LinuxGamepad.cpp:188 #15 (anonymous namespace)::LinuxGamepadService::Startup() (this=) at /home/emilio/src/moz/gecko-4/dom/gamepad/linux/LinuxGamepad.cpp:233 #16 mozilla::dom::StartGamepadMonitoring() () at /home/emilio/src/moz/gecko-4/dom/gamepad/linux/LinuxGamepad.cpp:334 #17 0x00007fd193269c6b in mozilla::dom::GamepadPlatformService::AddChannelParent(mozilla::dom::GamepadEventChannelParent*) (this=, aParent=) at /home/emilio/src/moz/gecko-4/dom/gamepad/GamepadPlatformService.cpp:225 #18 0x00007fd19326d175 in mozilla::dom::GamepadEventChannelParent::Init() (this=0x7fd136e76a00) at /home/emilio/src/moz/gecko-4/dom/gamepad/ipc/GamepadEventChannelParent.cpp:50 #19 0x00007fd1913ba3a6 in mozilla::ipc::BackgroundParentImpl::RecvPGamepadEventChannelConstructor(mozilla::dom::PGamepadEventChannelParent*) (this=0x7fd13f888000, aActor=0x0) at /home/emilio/src/moz/gecko-4/ipc/glue/BackgroundParentImpl.cpp:1109 #20 0x00007fd1917c7da1 in mozilla::ipc::PBackgroundParent::OnMessageReceived(IPC::Message const&) (this=0x7fd13f888000, msg__=...) at PBackgroundParent.cpp:4967 #21 0x00007fd1913ea71d in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) (this=0x7fd13f8880f8, aProxy=0x7fd13ff48140, aMsg=...) Differential Revision: https://phabricator.services.mozilla.com/D87967 --- dom/gamepad/GamepadPlatformService.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/dom/gamepad/GamepadPlatformService.cpp b/dom/gamepad/GamepadPlatformService.cpp index d3dfe04c845a5..ef93248feae84 100644 --- a/dom/gamepad/GamepadPlatformService.cpp +++ b/dom/gamepad/GamepadPlatformService.cpp @@ -207,20 +207,22 @@ void GamepadPlatformService::AddChannelParent( MOZ_ASSERT(!mChannelParents.Contains(aParent)); // We use mutex here to prevent race condition with monitor thread - MutexAutoLock autoLock(mMutex); - mChannelParents.AppendElement(aParent); - - // For a new GamepadEventChannel, we have to send the exising GamepadAdded - // to it to make it can have the same amount of gamepads with others. - if (mChannelParents.Length() > 1) { - for (const auto& evt : mGamepadAdded) { - GamepadChangeEventBody body(evt.second); - GamepadChangeEvent e(evt.first, GamepadServiceType::Standard, body); - aParent->DispatchUpdateEvent(e); + { + MutexAutoLock autoLock(mMutex); + mChannelParents.AppendElement(aParent); + + // For a new GamepadEventChannel, we have to send the exising GamepadAdded + // to it to make it can have the same amount of gamepads with others. + if (mChannelParents.Length() > 1) { + for (const auto& evt : mGamepadAdded) { + GamepadChangeEventBody body(evt.second); + GamepadChangeEvent e(evt.first, GamepadServiceType::Standard, body); + aParent->DispatchUpdateEvent(e); + } } - } - FlushPendingEvents(); + FlushPendingEvents(); + } StartGamepadMonitoring(); }