Skip to content

Commit f4ca7e7

Browse files
committed
Fix deadlock in SubprocessScheduler
Running the HHBBC compiler against a trivial test program in recent OSS HHVM hangs forever. LLDB shows workers deadlocking in SubprocessScheduler. One thread is waiting on `m_subprocessSchedulingCV` called when a subprocess is released: ``` * thread facebook#5, name = 'HPHPcWorker2' * frame #0: 0x00007ffff436d9a2 libc.so.6`__syscall_cancel_arch_end frame #1: 0x00007ffff4361c3c libc.so.6`__internal_syscall_cancel + 92 frame facebook#2: 0x00007ffff43622ac libc.so.6`__futex_abstimed_wait_common + 124 frame facebook#3: 0x00007ffff436497e libc.so.6`pthread_cond_wait@@GLIBC_2.3.2 + 334 frame facebook#4: 0x00007ffff46e60f3 libc++.so.1`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 19 frame facebook#5: 0x000000000069dccc hhvm`HPHP::extern_worker::SubprocessScheduler::acquire(this=0x00007fffeea62980, jobName="hhbbc-build-subclass") at subprocess-scheduler.cpp:56:30 frame facebook#6: 0x00000000006593b8 hhvm`HPHP::extern_worker::(anonymous namespace)::SubprocessImpl::exec(this=<unavailable>, requestId=<unavailable>, command=<unavailable>, config=<unavailable>, inputs=<unavailable>, output=<unavailable>, finiOutput=<unavailable>, (null)=<unavailable>) (.resume) at extern-worker.cpp:1540:44 frame facebook#7: 0x00000000042d1891 hhvm`std::__1::coroutine_handle<void>::resume[abi:se210108](this=<unavailable>) const at coroutine_handle.h:69:5 [inlined] frame facebook#8: 0x00000000042d1881 hhvm`folly::resumeCoroutineWithNewAsyncStackRoot(h=coro frame = 0x7fffcc625000, frame=<unavailable>) at AsyncStack.cpp:202:5 frame facebook#9: 0x000000000061e3b0 hhvm`folly::coro::detail::co_reschedule_on_current_executor_::StackAwareAwaiter::await_suspend_impl(std::__1::coroutine_handle<void>, folly::AsyncStackFrame&)::'lambda'()::operator()(this=0x00007fffd5f75270) at CurrentExecutor.h:92:15 frame facebook#10: 0x00000000041f54a2 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=0x00007fffd5f75270) at Function.h:370:12 [inlined] frame facebook#11: 0x00000000041f5499 hhvm`void folly::catch_exception<folly::Function<void ()>&, void (&)(char const*) noexcept, char const*&, void>(t=0x00007fffd5f75270, c=<unavailable>, a=<unavailable>) at Exception.h:359:12 [inlined] frame facebook#12: 0x00000000041f5499 hhvm`void folly::Executor::invokeCatchingExns<folly::Function<void ()>>(p=<unavailable>, f=Function<void ()> @ 0x00007fffd5f75270) at Executor.h:233:5 [inlined] frame facebook#13: 0x00000000041f5499 hhvm`folly::ThreadPoolExecutor::runTask(this=0x00007fffee6cc800, thread=std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>::element_type @ 0x00007fffd9217140, task=0x00007fffd5f75410) at ThreadPoolExecutor.cpp:142:7 frame facebook#14: 0x00000000041d3c9b hhvm`folly::CPUThreadPoolExecutor::threadRun(this=0x00007fffee6cc800, thread=<unavailable>) at CPUThreadPoolExecutor.cpp:337:5 frame facebook#15: 0x00000000041f7533 hhvm`std::__1::__invoke_result_impl<void, void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::type std::__1::__invoke[abi:se210108]<void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>(__args=<unavailable>, __args=<unavailable>, __args=<unavailable>) at invoke.h:87:27 [inlined] frame facebook#16: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:se210108]<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, 0ul, 1ul, std::__1::tuple<>>(__f=<unavailable>, __bound_args=<unavailable>, (null)=<unavailable>, __args=<unavailable>) at bind.h:195:10 [inlined] frame facebook#17: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::operator()[abi:se210108]<>(this=<unavailable>) at bind.h:222:12 [inlined] frame facebook#18: 0x00000000041f74fd hhvm`void folly::detail::function::call_<std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>, true, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#19: 0x0000000000642d11 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#20: 0x0000000000642d0d hhvm`HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=0x00007fffd9254050) at coro.cpp:53:9 [inlined] frame facebook#21: 0x0000000000642cf2 hhvm`void folly::detail::function::call_<HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'(), false, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#22: 0x0000000000644770 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#23: 0x000000000064476d hhvm`folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=<unavailable>) at NamedThreadFactory.h:40:11 [inlined] frame facebook#25: 0x000000000064476d hhvm`void std::__1::__thread_execute[abi:se210108]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>(__t=size=2, (null)=<unavailable>) at thread.h:159:3 [inlined] frame facebook#26: 0x0000000000644749 hhvm`void* std::__1::__thread_proxy[abi:se210108]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>>(__vp=0x00007fffd9259150) at thread.h:168:3 frame facebook#27: 0x00007ffff4365464 libc.so.6`start_thread + 740 frame facebook#28: 0x00007ffff43e85ec libc.so.6`__clone3 + 44 ``` However this worker does that while also holding `m_subprocessMapLock`, so the other worker can't insert a subprocess: ``` * thread facebook#3, name = 'HPHPcWorker0' * frame #0: 0x00007ffff43624e0 libc.so.6`__GI___lll_lock_wait + 48 frame #1: 0x00007ffff4368a31 libc.so.6`__pthread_mutex_lock@GLIBC_2.2.5 + 257 frame facebook#2: 0x00007ffff46e724a libc++.so.1`std::__1::mutex::lock() + 10 frame facebook#3: 0x000000000069e074 hhvm`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:se210108](this=<unavailable>, __m=0x00007fffeea629e8) at lock_guard.h:32:10 [inlined] frame facebook#4: 0x000000000069e06c hhvm`HPHP::extern_worker::SubprocessScheduler::registerSubprocess(this=0x00007fffeea62980, pid=35418, jobName=<unavailable>) at subprocess-scheduler.cpp:75:31 frame facebook#5: 0x0000000000657a40 hhvm`HPHP::extern_worker::(anonymous namespace)::SubprocessImpl::doSubprocess(this=<unavailable>, requestId=<unavailable>, command=<unavailable>, inputBlob=<unavailable>, outputPath=<unavailable>, usageEstimate=<unavailable>) (.resume) at extern-worker.cpp:1657:28 frame facebook#6: 0x00000000042d1891 hhvm`std::__1::coroutine_handle<void>::resume[abi:se210108](this=<unavailable>) const at coroutine_handle.h:69:5 [inlined] frame facebook#7: 0x00000000042d1881 hhvm`folly::resumeCoroutineWithNewAsyncStackRoot(h=coro frame = 0x7ffff33c4000, frame=<unavailable>) at AsyncStack.cpp:202:5 frame facebook#8: 0x000000000061e3b0 hhvm`folly::coro::detail::co_reschedule_on_current_executor_::StackAwareAwaiter::await_suspend_impl(std::__1::coroutine_handle<void>, folly::AsyncStackFrame&)::'lambda'()::operator()(this=0x00007fffdf6f4270) at CurrentExecutor.h:92:15 frame facebook#9: 0x00000000041f54a2 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=0x00007fffdf6f4270) at Function.h:370:12 [inlined] frame facebook#10: 0x00000000041f5499 hhvm`void folly::catch_exception<folly::Function<void ()>&, void (&)(char const*) noexcept, char const*&, void>(t=0x00007fffdf6f4270, c=<unavailable>, a=<unavailable>) at Exception.h:359:12 [inlined] frame facebook#11: 0x00000000041f5499 hhvm`void folly::Executor::invokeCatchingExns<folly::Function<void ()>>(p=<unavailable>, f=Function<void ()> @ 0x00007fffdf6f4270) at Executor.h:233:5 [inlined] frame facebook#12: 0x00000000041f5499 hhvm`folly::ThreadPoolExecutor::runTask(this=0x00007fffee6cc800, thread=std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>::element_type @ 0x00007fffdff21240, task=0x00007fffdf6f4410) at ThreadPoolExecutor.cpp:142:7 frame facebook#13: 0x00000000041d3c9b hhvm`folly::CPUThreadPoolExecutor::threadRun(this=0x00007fffee6cc800, thread=<unavailable>) at CPUThreadPoolExecutor.cpp:337:5 frame facebook#14: 0x00000000041f7533 hhvm`std::__1::__invoke_result_impl<void, void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::type std::__1::__invoke[abi:se210108]<void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>(__args=<unavailable>, __args=<unavailable>, __args=<unavailable>) at invoke.h:87:27 [inlined] frame facebook#15: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:se210108]<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, 0ul, 1ul, std::__1::tuple<>>(__f=<unavailable>, __bound_args=<unavailable>, (null)=<unavailable>, __args=<unavailable>) at bind.h:195:10 [inlined] frame facebook#16: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::operator()[abi:se210108]<>(this=<unavailable>) at bind.h:222:12 [inlined] frame facebook#17: 0x00000000041f74fd hhvm`void folly::detail::function::call_<std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>, true, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#18: 0x0000000000642d11 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#19: 0x0000000000642d0d hhvm`HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=0x00007fffeeacb240) at coro.cpp:53:9 [inlined] frame facebook#20: 0x0000000000642cf2 hhvm`void folly::detail::function::call_<HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'(), false, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#21: 0x0000000000644770 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#22: 0x000000000064476d hhvm`folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=<unavailable>) at NamedThreadFactory.h:40:11 [inlined] frame facebook#24: 0x000000000064476d hhvm`void std::__1::__thread_execute[abi:se210108]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>(__t=size=2, (null)=<unavailable>) at thread.h:159:3 [inlined] frame facebook#25: 0x0000000000644749 hhvm`void* std::__1::__thread_proxy[abi:se210108]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>>(__vp=0x00007fffeea23d40) at thread.h:168:3 frame facebook#26: 0x00007ffff4365464 libc.so.6`start_thread + 740 frame facebook#27: 0x00007ffff43e85ec libc.so.6`__clone3 + 44 ``` So release `m_subprocessMapLock` in `acquire()` once we're done reading the map so that other threads get a chance to write it.
1 parent 242c9a8 commit f4ca7e7

1 file changed

Lines changed: 10 additions & 8 deletions

File tree

hphp/util/subprocess-scheduler.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,16 @@ SubprocessScheduler::SubprocessScheduler(uint64_t maxMemoryUsage)
4343

4444
uint64_t SubprocessScheduler::acquire(const std::string& jobName) {
4545
do {
46-
std::lock_guard<std::mutex> _{m_subprocessMapLock};
47-
uint64_t usageEstimate = getRSSEstimate(jobName);
48-
if (usageEstimate == 0 ||
49-
m_estimatedMemoryUsage + usageEstimate <= m_maxMemoryUsage) {
50-
FTRACE(3, "Scheduling `{}` job ({} running). est. job RSS: {}, est. total memory usage: {}\n",
51-
jobName, m_runningSubprocesses.size(), usageEstimate, m_estimatedMemoryUsage);
52-
m_estimatedMemoryUsage += usageEstimate;
53-
return usageEstimate;
46+
{
47+
std::lock_guard<std::mutex> _{m_subprocessMapLock};
48+
uint64_t usageEstimate = getRSSEstimate(jobName);
49+
if (usageEstimate == 0 ||
50+
m_estimatedMemoryUsage + usageEstimate <= m_maxMemoryUsage) {
51+
FTRACE(3, "Scheduling `{}` job ({} running). est. job RSS: {}, est. total memory usage: {}\n",
52+
jobName, m_runningSubprocesses.size(), usageEstimate, m_estimatedMemoryUsage);
53+
m_estimatedMemoryUsage += usageEstimate;
54+
return usageEstimate;
55+
}
5456
}
5557
std::unique_lock<std::mutex> lock(m_subprocessSchedulingLock);
5658
m_subprocessSchedulingCV.wait(lock);

0 commit comments

Comments
 (0)