summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom <tomut@yahoo.com>2020-12-08 19:04:05 -0700
committerAndreas Kling <kling@serenityos.org>2020-12-12 21:28:12 +0100
commit4bbee00650ab6f79d79dd6ec155563d6b4dc29dd (patch)
tree6c15a686f81b30276a9f417ed5c0c48f3bb51686
parentbcb9363a97ca4d79bdd75d609ec6cdcc9e5f0d81 (diff)
downloadserenity-4bbee00650ab6f79d79dd6ec155563d6b4dc29dd.zip
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.
-rw-r--r--Kernel/Process.cpp5
-rw-r--r--Kernel/Process.h1
-rw-r--r--Kernel/Syscalls/disown.cpp1
-rw-r--r--Kernel/Thread.h5
-rw-r--r--Kernel/ThreadBlockers.cpp44
5 files changed, 55 insertions, 1 deletions
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index d934791be8..620a17c615 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -642,6 +642,11 @@ void Process::finalize(Thread& last_thread)
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)
{
if (auto parent = Process::from_pid(ppid()))
diff --git a/Kernel/Process.h b/Kernel/Process.h
index 94c8181cf6..03f543d14d 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -488,6 +488,7 @@ public:
KResultOr<u32> peek_user_data(Userspace<const u32*> address);
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);
Thread::WaitBlockCondition& wait_block_condition() { return m_wait_block_condition; }
diff --git a/Kernel/Syscalls/disown.cpp b/Kernel/Syscalls/disown.cpp
index 6c6368aecd..3c7ddf5eab 100644
--- a/Kernel/Syscalls/disown.cpp
+++ b/Kernel/Syscalls/disown.cpp
@@ -37,6 +37,7 @@ int Process::sys$disown(ProcessID pid)
if (process->ppid() != this->pid())
return -ECHILD;
process->m_ppid = 0;
+ process->disowned_by_waiter(*this);
return 0;
}
}
diff --git a/Kernel/Thread.h b/Kernel/Thread.h
index 4086d40621..77aa4940c0 100644
--- a/Kernel/Thread.h
+++ b/Kernel/Thread.h
@@ -644,7 +644,8 @@ public:
enum class UnblockFlags {
Terminated,
Stopped,
- Continued
+ Continued,
+ Disowned
};
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); }
private:
+ void do_was_disowned();
void do_set_result(const siginfo_t&);
const int m_wait_options;
@@ -681,6 +683,7 @@ public:
{
}
+ void disowned_by_waiter(Process&);
bool unblock(Thread&, WaitBlocker::UnblockFlags, u8);
void try_unblock(WaitBlocker&);
void finalize();
diff --git a/Kernel/ThreadBlockers.cpp b/Kernel/ThreadBlockers.cpp
index ae31751bbe..c46ef77397 100644
--- a/Kernel/ThreadBlockers.cpp
+++ b/Kernel/ThreadBlockers.cpp
@@ -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)
{
+ ASSERT(flags != WaitBlocker::UnblockFlags::Disowned);
+
bool did_unblock_any = false;
bool did_wait = false;
bool was_waited_already = false;
@@ -565,6 +590,13 @@ void Thread::WaitBlocker::was_unblocked(bool)
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)
{
ASSERT(!m_did_unblock);
@@ -599,6 +631,10 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
return false;
break;
case P_ALL:
+ if (flags == UnblockFlags::Disowned) {
+ // Generic waiter won't be unblocked by disown
+ return false;
+ }
break;
default:
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())
return false;
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) {
@@ -645,6 +687,7 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
switch (flags) {
case UnblockFlags::Terminated:
+ case UnblockFlags::Disowned:
ASSERT_NOT_REACHED();
case UnblockFlags::Stopped:
siginfo.si_code = CLD_STOPPED;
@@ -665,6 +708,7 @@ bool Thread::WaitBlocker::unblock(Thread& thread, UnblockFlags flags, u8 signal,
if (!from_add_blocker) {
// Only call unblock if we weren't called from within set_block_condition!
+ ASSERT(flags != UnblockFlags::Disowned);
unblock_from_blocker();
}
// Because this may be called from add_blocker, in which case we should