Kernel: disown should unblock any potential waiters

This is necessary because if a process changes the state to Stopped
or resumes from that state, a wait entry is created in the parent
process. So, if a child process does this before disown is called,
we need to clear those entries to avoid leaking references/zombies
that won't be cleaned up until the former parent exits.

This also should solve an even more unlikely corner case where another
thread is waiting on a pid that is being disowned by another thread.
This commit is contained in:
Tom 2020-12-08 19:04:05 -07:00 committed by Andreas Kling
parent bcb9363a97
commit 4bbee00650
Notes: sideshowbarker 2024-07-19 00:54:34 +09:00
5 changed files with 55 additions and 1 deletions

View file

@ -642,6 +642,11 @@ void Process::finalize(Thread& last_thread)
m_wait_block_condition.finalize(); m_wait_block_condition.finalize();
} }
void Process::disowned_by_waiter(Process& process)
{
m_wait_block_condition.disowned_by_waiter(process);
}
void Process::unblock_waiters(Thread& thread, Thread::WaitBlocker::UnblockFlags flags, u8 signal) void Process::unblock_waiters(Thread& thread, Thread::WaitBlocker::UnblockFlags flags, u8 signal)
{ {
if (auto parent = Process::from_pid(ppid())) if (auto parent = Process::from_pid(ppid()))

View file

@ -488,6 +488,7 @@ public:
KResultOr<u32> peek_user_data(Userspace<const u32*> address); KResultOr<u32> peek_user_data(Userspace<const u32*> address);
KResult poke_user_data(Userspace<u32*> address, u32 data); KResult poke_user_data(Userspace<u32*> address, u32 data);
void disowned_by_waiter(Process& process);
void unblock_waiters(Thread&, Thread::WaitBlocker::UnblockFlags, u8 signal = 0); void unblock_waiters(Thread&, Thread::WaitBlocker::UnblockFlags, u8 signal = 0);
Thread::WaitBlockCondition& wait_block_condition() { return m_wait_block_condition; } Thread::WaitBlockCondition& wait_block_condition() { return m_wait_block_condition; }

View file

@ -37,6 +37,7 @@ int Process::sys$disown(ProcessID pid)
if (process->ppid() != this->pid()) if (process->ppid() != this->pid())
return -ECHILD; return -ECHILD;
process->m_ppid = 0; process->m_ppid = 0;
process->disowned_by_waiter(*this);
return 0; return 0;
} }
} }

View file

@ -644,7 +644,8 @@ public:
enum class UnblockFlags { enum class UnblockFlags {
Terminated, Terminated,
Stopped, Stopped,
Continued Continued,
Disowned
}; };
WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr<siginfo_t>& result); WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr<siginfo_t>& result);
@ -658,6 +659,7 @@ public:
bool is_wait() const { return !(m_wait_options & WNOWAIT); } bool is_wait() const { return !(m_wait_options & WNOWAIT); }
private: private:
void do_was_disowned();
void do_set_result(const siginfo_t&); void do_set_result(const siginfo_t&);
const int m_wait_options; const int m_wait_options;
@ -681,6 +683,7 @@ public:
{ {
} }
void disowned_by_waiter(Process&);
bool unblock(Thread&, WaitBlocker::UnblockFlags, u8); bool unblock(Thread&, WaitBlocker::UnblockFlags, u8);
void try_unblock(WaitBlocker&); void try_unblock(WaitBlocker&);
void finalize(); void finalize();

View file

@ -412,8 +412,33 @@ void Thread::WaitBlockCondition::try_unblock(Thread::WaitBlocker& blocker)
} }
} }
void Thread::WaitBlockCondition::disowned_by_waiter(Process& process)
{
ScopedSpinLock lock(m_lock);
if (m_finalized)
return;
for (size_t i = 0; i < m_threads.size();) {
auto& info = m_threads[i];
if (&info.thread->process() == &process) {
do_unblock([&](Blocker& b, void*) {
ASSERT(b.blocker_type() == Blocker::Type::Wait);
auto& blocker = static_cast<WaitBlocker&>(b);
bool did_unblock = blocker.unblock(info.thread, WaitBlocker::UnblockFlags::Disowned, 0, false);
ASSERT(did_unblock); // disowning must unblock everyone
return true;
});
m_threads.remove(i);
continue;
}
i++;
}
}
bool Thread::WaitBlockCondition::unblock(Thread& thread, WaitBlocker::UnblockFlags flags, u8 signal) bool Thread::WaitBlockCondition::unblock(Thread& thread, WaitBlocker::UnblockFlags flags, u8 signal)
{ {
ASSERT(flags != WaitBlocker::UnblockFlags::Disowned);
bool did_unblock_any = false; bool did_unblock_any = false;
bool did_wait = false; bool did_wait = false;
bool was_waited_already = false; bool was_waited_already = false;
@ -565,6 +590,13 @@ void Thread::WaitBlocker::was_unblocked(bool)
current_thread->try_dispatch_one_pending_signal(SIGCHLD); current_thread->try_dispatch_one_pending_signal(SIGCHLD);
} }
void Thread::WaitBlocker::do_was_disowned()
{
ASSERT(!m_did_unblock);
m_did_unblock = true;
m_result = KResult(-ECHILD);
}
void Thread::WaitBlocker::do_set_result(const siginfo_t& result) void Thread::WaitBlocker::do_set_result(const siginfo_t& result)
{ {
ASSERT(!m_did_unblock); ASSERT(!m_did_unblock);
@ -599,6 +631,10 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
return false; return false;
break; break;
case P_ALL: case P_ALL:
if (flags == UnblockFlags::Disowned) {
// Generic waiter won't be unblocked by disown
return false;
}
break; break;
default: default:
ASSERT_NOT_REACHED(); ASSERT_NOT_REACHED();
@ -621,6 +657,12 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
if (!(m_wait_options & WUNTRACED) && !thread.is_traced()) if (!(m_wait_options & WUNTRACED) && !thread.is_traced())
return false; return false;
break; break;
case UnblockFlags::Disowned:
ScopedSpinLock lock(m_lock);
// Disowning must unblock anyone waiting for this process explicitly
if (!m_did_unblock)
do_was_disowned();
return true;
} }
if (flags == UnblockFlags::Terminated) { if (flags == UnblockFlags::Terminated) {
@ -645,6 +687,7 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
switch (flags) { switch (flags) {
case UnblockFlags::Terminated: case UnblockFlags::Terminated:
case UnblockFlags::Disowned:
ASSERT_NOT_REACHED(); ASSERT_NOT_REACHED();
case UnblockFlags::Stopped: case UnblockFlags::Stopped:
siginfo.si_code = CLD_STOPPED; siginfo.si_code = CLD_STOPPED;
@ -665,6 +708,7 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
if (!from_add_blocker) { if (!from_add_blocker) {
// Only call unblock if we weren't called from within set_block_condition! // Only call unblock if we weren't called from within set_block_condition!
ASSERT(flags != UnblockFlags::Disowned);
unblock_from_blocker(); unblock_from_blocker();
} }
// Because this may be called from add_blocker, in which case we should // Because this may be called from add_blocker, in which case we should