From 133082ce5ac6d4ed0341802d67050aaac8f3a986 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 20 Aug 2026 10:53:51 +0800 Subject: [PATCH 1/6] FIX: avoid polling captured subprocesses --- CHANGELOG.md | 6 +++ lib/landlock/process_io.rb | 93 +++++++++++++++++++++-------------- lib/landlock/version.rb | 2 +- test/landlock_capture_test.rb | 68 +++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e28744..cdc96e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ## Unreleased +## [0.4.1] - 2026-08-20 + +### Fixed + +- Remove the 100 ms child-exit polling interval from subprocess capture. Capture now drains stdout and stderr before waiting directly for the child, while preserving wall-clock timeout enforcement after both streams close. + ## [0.4] - 2026-08-10 ### Changed diff --git a/lib/landlock/process_io.rb b/lib/landlock/process_io.rb index 904dd94..e4ea501 100644 --- a/lib/landlock/process_io.rb +++ b/lib/landlock/process_io.rb @@ -5,7 +5,6 @@ module Landlock READ_CHUNK_BYTES = 16 * 1024 - PROCESS_POLL_SECONDS = 0.1 STDIN_THREAD_JOIN_SECONDS = 0.1 POST_TIMEOUT_DRAIN_SECONDS = 0.05 @@ -91,45 +90,16 @@ def read_and_wait(pid, streams, timeout, max_output_bytes, truncate_output, stat timed_out = false status = nil - until streams.empty? && status + until streams.empty? if deadline remaining = deadline - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) if remaining <= 0 timed_out = true - terminate_process(pid) - status = wait_for_pid(pid) - drain_streams_until( - streams, - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + POST_TIMEOUT_DRAIN_SECONDS, - max_output_bytes, - truncate_output, - state, - pid - ) - close_streams(streams) break end end - status ||= poll_pid(pid) - - break if streams.empty? && status - - wait = - ( - if deadline - [deadline - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC), PROCESS_POLL_SECONDS].min - else - PROCESS_POLL_SECONDS - end - ) - wait = 0 if wait.negative? - if streams.empty? - sleep wait - next - end - - readable, = IO.select(streams.keys, nil, nil, wait) + readable, = IO.select(streams.keys, nil, nil, remaining) next unless readable readable.each do |io| @@ -145,16 +115,63 @@ def read_and_wait(pid, streams, timeout, max_output_bytes, truncate_output, stat end end - status ||= wait_for_pid(pid) + if deadline + status, timed_out = wait_for_pid_until(pid, deadline:) + else + status = wait_for_pid(pid) + end + + if timed_out + drain_streams_until( + streams, + ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + POST_TIMEOUT_DRAIN_SECONDS, + max_output_bytes, + truncate_output, + state, + pid + ) + close_streams(streams) + end + [status, timed_out] end - def poll_pid(pid) - result = ::Process.wait2(pid, ::Process::WNOHANG) - result&.last - rescue Errno::ECHILD - nil + def wait_for_pid_until(pid, deadline:) + remaining = deadline - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + if remaining <= 0 + terminate_process(pid) + return wait_for_pid(pid), true + end + + mutex = Mutex.new + timed_out = false + timeout_thread = + Thread.new do + sleep remaining + mutex.synchronize { timed_out = true } + terminate_process(pid) + end + + begin + status = wait_for_pid(pid) + timeout_started = + mutex.synchronize do + if timed_out + true + else + timeout_thread.kill + false + end + end + timeout_started ? timeout_thread.value : timeout_thread.join + + [status, timeout_started] + ensure + timeout_thread.kill + timeout_thread.join + end end + private_class_method :wait_for_pid_until def wait_for_pid(pid) ::Process.wait2(pid).last diff --git a/lib/landlock/version.rb b/lib/landlock/version.rb index 1ec5bbf..2b1ac44 100644 --- a/lib/landlock/version.rb +++ b/lib/landlock/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Landlock - VERSION = "0.4" + VERSION = "0.4.1" end diff --git a/test/landlock_capture_test.rb b/test/landlock_capture_test.rb index b2d796b..db469ec 100644 --- a/test/landlock_capture_test.rb +++ b/test/landlock_capture_test.rb @@ -105,6 +105,74 @@ def test_capture_does_not_false_timeout_after_streams_close refute result.timed_out? end + def test_capture_waits_for_child_exit_without_polling_after_streams_close + skip "Landlock unsupported" unless Landlock.supported? + + result = nil + Landlock::ProcessIO.stub(:sleep, ->(*) { flunk "capture polled for child exit" }) do + result = + Landlock.capture( + [RbConfig.ruby, "--disable=gems", "-e", "STDOUT.close; STDERR.close; sleep 0.25"], + rlimits: { + open_files: 64 + } + ) + end + + assert result.status.success? + refute result.timed_out? + end + + def test_capture_timeout_applies_after_streams_close + skip "Landlock unsupported" unless Landlock.supported? + + error = + assert_raises(Landlock::CommandError) do + Landlock.capture!( + ["/bin/sh", "-c", "exec 1>&- 2>&-; exec /bin/sleep 30"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + + assert error.result.timed_out? + refute_nil error.status + assert error.status.signaled? + end + + def test_capture_cancels_timeout_when_waiting_for_child_raises + skip "Landlock unsupported" unless Landlock.supported? + + timeout_threads = [] + wait_calls = 0 + original_thread_new = Thread.method(:new) + original_wait_for_pid = Landlock::ProcessIO.method(:wait_for_pid) + thread_new = + lambda do |*arguments, &block| + original_thread_new.call(*arguments, &block).tap { |thread| timeout_threads << thread } + end + wait_for_pid = + lambda do |pid| + wait_calls += 1 + raise IOError, "wait failed" if wait_calls == 1 + + original_wait_for_pid.call(pid) + end + + Thread.stub(:new, thread_new) do + Landlock::ProcessIO.stub(:wait_for_pid, wait_for_pid) do + assert_raises(IOError) do + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 30"], rlimits: { open_files: 64 }, timeout: 10) + end + end + end + + assert_equal 1, timeout_threads.size + refute timeout_threads.first.alive? + end + def test_capture_does_not_wait_forever_for_blocked_stdin_reader skip "Landlock unsupported" unless Landlock.supported? From b2534ea132d00622ace1f740c9ad115a20bf1ae0 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 20 Aug 2026 15:28:52 +1000 Subject: [PATCH 2/6] FIX: monitor captured subprocesses with pidfds Use pidfds to wait for captured children after their output streams close, avoiding timeout threads while preserving wall-clock deadline enforcement. Fall back to bounded polling when pidfds are unavailable, and ensure pidfd resources are closed on success and error paths. --- CHANGELOG.md | 2 +- ext/landlock/landlock.c | 15 ++++ ext/landlock/landlock_native.h | 4 + lib/landlock/native.rb | 4 + lib/landlock/process_io.rb | 61 ++++++++------- test/landlock_capture_test.rb | 131 ++++++++++++++++++++++++++++----- 6 files changed, 171 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdc96e6..8f5b38b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to this project will be documented in this file. ### Fixed -- Remove the 100 ms child-exit polling interval from subprocess capture. Capture now drains stdout and stderr before waiting directly for the child, while preserving wall-clock timeout enforcement after both streams close. +- Remove the 100 ms child-exit polling interval from subprocess capture. Capture now drains stdout and stderr, then monitors the child via pidfd until exit or the wall-clock deadline, retaining polling only as a compatibility fallback when pidfds are unavailable. ## [0.4] - 2026-08-10 diff --git a/ext/landlock/landlock.c b/ext/landlock/landlock.c index 63a2026..55b8f43 100644 --- a/ext/landlock/landlock.c +++ b/ext/landlock/landlock.c @@ -121,6 +121,20 @@ static VALUE rb_ll_close_fd(VALUE self, VALUE fd_value) { return Qnil; } +static VALUE rb_ll_pidfd_open(VALUE self, VALUE pid_value) { +#ifdef SYS_pidfd_open + int fd = syscall(SYS_pidfd_open, NUM2PIDT(pid_value), 0); + if (fd < 0) { + raise_syscall_error("pidfd_open"); + } + return INT2NUM(fd); +#else + errno = ENOSYS; + raise_syscall_error("pidfd_open"); + return Qnil; +#endif +} + static VALUE rb_ll_seccomp_deny_network(VALUE self) { const char *error_message = "seccomp(SECCOMP_SET_MODE_FILTER)"; if (rb_landlock_seccomp_deny_network(&error_message) != 0) { @@ -150,6 +164,7 @@ void Init_landlock(void) { rb_define_singleton_method(mLandlock, "_add_net_rule", rb_ll_add_net_rule, 3); rb_define_singleton_method(mLandlock, "_restrict_self", rb_ll_restrict_self, 1); rb_define_singleton_method(mLandlock, "_close_fd", rb_ll_close_fd, 1); + rb_define_singleton_method(mLandlock, "_pidfd_open", rb_ll_pidfd_open, 1); rb_define_singleton_method(mLandlock, "seccomp_deny_network!", rb_ll_seccomp_deny_network, 0); rb_define_const(mLandlock, "ACCESS_FS_EXECUTE", ULL2NUM(LANDLOCK_ACCESS_FS_EXECUTE)); diff --git a/ext/landlock/landlock_native.h b/ext/landlock/landlock_native.h index d92000d..1532531 100644 --- a/ext/landlock/landlock_native.h +++ b/ext/landlock/landlock_native.h @@ -35,6 +35,10 @@ #endif #endif +#if defined(__linux__) && !defined(SYS_pidfd_open) && defined(__NR_pidfd_open) +#define SYS_pidfd_open __NR_pidfd_open +#endif + #ifndef LANDLOCK_CREATE_RULESET_VERSION #define LANDLOCK_CREATE_RULESET_VERSION (1U << 0) #endif diff --git a/lib/landlock/native.rb b/lib/landlock/native.rb index 6035f17..546c9c1 100644 --- a/lib/landlock/native.rb +++ b/lib/landlock/native.rb @@ -31,6 +31,10 @@ def close_fd(fd) Landlock.__send__(:_close_fd, fd) end + def pidfd_open(pid) + Landlock.__send__(:_pidfd_open, pid) + end + def seccomp_deny_network! Landlock.seccomp_deny_network! end diff --git a/lib/landlock/process_io.rb b/lib/landlock/process_io.rb index e4ea501..bf7eb4b 100644 --- a/lib/landlock/process_io.rb +++ b/lib/landlock/process_io.rb @@ -5,6 +5,7 @@ module Landlock READ_CHUNK_BYTES = 16 * 1024 + PID_WAIT_FALLBACK_INTERVAL_SECONDS = 0.1 STDIN_THREAD_JOIN_SECONDS = 0.1 POST_TIMEOUT_DRAIN_SECONDS = 0.05 @@ -137,41 +138,51 @@ def read_and_wait(pid, streams, timeout, max_output_bytes, truncate_output, stat end def wait_for_pid_until(pid, deadline:) - remaining = deadline - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + remaining = deadline - monotonic_time if remaining <= 0 terminate_process(pid) return wait_for_pid(pid), true end - mutex = Mutex.new - timed_out = false - timeout_thread = - Thread.new do - sleep remaining - mutex.synchronize { timed_out = true } + pidfd = Native.pidfd_open(pid) + pid_monitor = IO.for_fd(pidfd, autoclose: false) + readable, = IO.select([pid_monitor], nil, nil, remaining) + unless readable + terminate_process(pid) + return wait_for_pid(pid), true + end + + [wait_for_pid(pid), false] + rescue Landlock::SyscallError + wait_for_pid_until_by_polling(pid, deadline:) + ensure + close_stream(pid_monitor) if pid_monitor + Native.close_fd(pidfd) if pidfd + end + private_class_method :wait_for_pid_until + + def wait_for_pid_until_by_polling(pid, deadline:) + loop do + result = ::Process.wait2(pid, ::Process::WNOHANG) + return result.last, false if result + + remaining = deadline - monotonic_time + if remaining <= 0 terminate_process(pid) + return wait_for_pid(pid), true end - begin - status = wait_for_pid(pid) - timeout_started = - mutex.synchronize do - if timed_out - true - else - timeout_thread.kill - false - end - end - timeout_started ? timeout_thread.value : timeout_thread.join - - [status, timeout_started] - ensure - timeout_thread.kill - timeout_thread.join + IO.select(nil, nil, nil, [remaining, PID_WAIT_FALLBACK_INTERVAL_SECONDS].min) end + rescue Errno::ECHILD + [nil, false] end - private_class_method :wait_for_pid_until + private_class_method :wait_for_pid_until_by_polling + + def monotonic_time + ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + end + private_class_method :monotonic_time def wait_for_pid(pid) ::Process.wait2(pid).last diff --git a/test/landlock_capture_test.rb b/test/landlock_capture_test.rb index db469ec..0c686f5 100644 --- a/test/landlock_capture_test.rb +++ b/test/landlock_capture_test.rb @@ -126,33 +126,52 @@ def test_capture_waits_for_child_exit_without_polling_after_streams_close def test_capture_timeout_applies_after_streams_close skip "Landlock unsupported" unless Landlock.supported? - error = - assert_raises(Landlock::CommandError) do - Landlock.capture!( - ["/bin/sh", "-c", "exec 1>&- 2>&-; exec /bin/sleep 30"], + error = nil + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + error = + assert_raises(Landlock::CommandError) do + Landlock.capture!( + ["/bin/sh", "-c", "exec 1>&- 2>&-; exec /bin/sleep 30"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + end + + assert error.result.timed_out? + refute_nil error.status + assert error.status.signaled? + end + + def test_capture_does_not_create_timeout_thread_after_streams_close + skip "Landlock unsupported" unless Landlock.supported? + + result = nil + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + result = + Landlock.capture( + [RbConfig.ruby, "--disable=gems", "-e", "STDOUT.close; STDERR.close; sleep 0.1"], rlimits: { open_files: 64 }, - timeout: 0.1 + timeout: 5 ) - end + end - assert error.result.timed_out? - refute_nil error.status - assert error.status.signaled? + assert result.status.success? + refute result.timed_out? end - def test_capture_cancels_timeout_when_waiting_for_child_raises + def test_capture_closes_pid_monitor_when_waiting_for_child_raises skip "Landlock unsupported" unless Landlock.supported? - timeout_threads = [] + pid_monitors = [] wait_calls = 0 - original_thread_new = Thread.method(:new) + original_for_fd = IO.method(:for_fd) original_wait_for_pid = Landlock::ProcessIO.method(:wait_for_pid) - thread_new = - lambda do |*arguments, &block| - original_thread_new.call(*arguments, &block).tap { |thread| timeout_threads << thread } - end + for_fd = ->(*arguments, **options) { original_for_fd.call(*arguments, **options).tap { |io| pid_monitors << io } } wait_for_pid = lambda do |pid| wait_calls += 1 @@ -161,16 +180,88 @@ def test_capture_cancels_timeout_when_waiting_for_child_raises original_wait_for_pid.call(pid) end - Thread.stub(:new, thread_new) do + IO.stub(:for_fd, for_fd) do Landlock::ProcessIO.stub(:wait_for_pid, wait_for_pid) do assert_raises(IOError) do - Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 30"], rlimits: { open_files: 64 }, timeout: 10) + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.1"], rlimits: { open_files: 64 }, timeout: 10) + end + end + end + + assert_equal 1, pid_monitors.size + assert_predicate pid_monitors.first, :closed? + end + + def test_capture_closes_raw_pidfd_when_wrapping_it_raises + skip "Landlock unsupported" unless Landlock.supported? + + pidfd = nil + closed_pidfds = [] + original_pidfd_open = Landlock::Native.method(:pidfd_open) + original_close_fd = Landlock::Native.method(:close_fd) + pidfd_open = ->(pid) { original_pidfd_open.call(pid).tap { |fd| pidfd = fd } } + close_fd = + lambda do |fd| + closed_pidfds << fd if fd == pidfd + original_close_fd.call(fd) + end + + Landlock::Native.stub(:pidfd_open, pidfd_open) do + Landlock::Native.stub(:close_fd, close_fd) do + IO.stub(:for_fd, ->(*) { raise IOError, "wrap failed" }) do + assert_raises(IOError) do + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 30"], rlimits: { open_files: 64 }, timeout: 10) + end end end end - assert_equal 1, timeout_threads.size - refute timeout_threads.first.alive? + refute_nil pidfd + assert_equal [pidfd], closed_pidfds + end + + def test_capture_falls_back_without_a_timeout_thread_when_pidfd_is_unavailable + skip "Landlock unsupported" unless Landlock.supported? + + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::EPERM::Errno) + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + result = + Landlock.capture( + [RbConfig.ruby, "--disable=gems", "-e", "STDOUT.close; STDERR.close; sleep 0.1"], + rlimits: { + open_files: 64 + }, + timeout: 5 + ) + end + end + + assert result.status.success? + refute result.timed_out? + end + + def test_capture_fallback_enforces_timeout_when_pidfd_is_unavailable + skip "Landlock unsupported" unless Landlock.supported? + + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + result = + Landlock.capture( + ["/bin/sh", "-c", "exec 1>&- 2>&-; exec /bin/sleep 30"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + end + + assert result.timed_out? + assert_predicate result.status, :signaled? end def test_capture_does_not_wait_forever_for_blocked_stdin_reader From 00881980b9614e3544685479e673025347e7e842 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 20 Aug 2026 14:44:10 +0800 Subject: [PATCH 3/6] FIX: Preserve capture deadlines after process readiness A pidfd or polling wait could observe child completion after its wall-clock deadline and still report success when the parent resumed late. This commit rechecks the monotonic deadline before accepting either readiness path. Late completions remain timed out and trigger process-group cleanup, while on-time exits retain the direct wait behavior. --- lib/landlock/process_io.rb | 16 ++++++--- test/landlock_capture_test.rb | 67 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/lib/landlock/process_io.rb b/lib/landlock/process_io.rb index bf7eb4b..4b34982 100644 --- a/lib/landlock/process_io.rb +++ b/lib/landlock/process_io.rb @@ -147,7 +147,7 @@ def wait_for_pid_until(pid, deadline:) pidfd = Native.pidfd_open(pid) pid_monitor = IO.for_fd(pidfd, autoclose: false) readable, = IO.select([pid_monitor], nil, nil, remaining) - unless readable + if !readable || monotonic_time >= deadline terminate_process(pid) return wait_for_pid(pid), true end @@ -163,15 +163,23 @@ def wait_for_pid_until(pid, deadline:) def wait_for_pid_until_by_polling(pid, deadline:) loop do - result = ::Process.wait2(pid, ::Process::WNOHANG) - return result.last, false if result - remaining = deadline - monotonic_time if remaining <= 0 terminate_process(pid) return wait_for_pid(pid), true end + result = ::Process.wait2(pid, ::Process::WNOHANG) + if result + status = result.last + if monotonic_time >= deadline + terminate_process(pid) + return status, true + end + + return status, false + end + IO.select(nil, nil, nil, [remaining, PID_WAIT_FALLBACK_INTERVAL_SECONDS].min) end rescue Errno::ECHILD diff --git a/test/landlock_capture_test.rb b/test/landlock_capture_test.rb index 0c686f5..97e603e 100644 --- a/test/landlock_capture_test.rb +++ b/test/landlock_capture_test.rb @@ -145,6 +145,73 @@ def test_capture_timeout_applies_after_streams_close assert error.status.signaled? end + def test_capture_rechecks_deadline_after_pidfd_becomes_readable + skip "Landlock unsupported" unless Landlock.supported? + + pid_monitors = [] + original_for_fd = IO.method(:for_fd) + original_select = IO.method(:select) + for_fd = ->(*arguments, **options) { original_for_fd.call(*arguments, **options).tap { |io| pid_monitors << io } } + select = + lambda do |readers, writers = nil, errors = nil, timeout = nil| + if readers&.any? { |io| pid_monitors.include?(io) } + original_select.call(readers, writers, errors, 5) + else + original_select.call(readers, writers, errors, timeout) + end + end + + result = nil + IO.stub(:for_fd, for_fd) do + IO.stub(:select, select) do + result = + Landlock.capture( + ["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.15"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + end + + assert_predicate result, :timed_out? + assert_predicate result.status, :success? + end + + def test_capture_rechecks_deadline_before_accepting_polled_status + skip "Landlock unsupported" unless Landlock.supported? + + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) + original_select = IO.method(:select) + select = + lambda do |readers, writers = nil, errors = nil, timeout = nil| + if readers.nil? && writers.nil? && errors.nil? + sleep 1 + nil + else + original_select.call(readers, writers, errors, timeout) + end + end + + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + IO.stub(:select, select) do + result = + Landlock.capture( + ["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.15"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + end + + assert_predicate result, :timed_out? + assert_predicate result.status, :success? + end + def test_capture_does_not_create_timeout_thread_after_streams_close skip "Landlock unsupported" unless Landlock.supported? From 640f72e81cd6c7611275883d96cb2f96e80f44d4 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 20 Aug 2026 15:11:22 +0800 Subject: [PATCH 4/6] FIX: Avoid signaling reaped capture PIDs The polling fallback can cross its deadline while wait2 reaps the child. Signaling the process group afterward risks targeting an unrelated process if the PID has already been reused.\n\nKeep the late result marked as timed out, but do not terminate after reaping it. Exercise the exact deadline crossing through the public capture API and verify that no signal follows the reap. --- lib/landlock/process_io.rb | 7 +----- test/landlock_capture_test.rb | 46 +++++++++++++++-------------------- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/lib/landlock/process_io.rb b/lib/landlock/process_io.rb index 4b34982..ddd1733 100644 --- a/lib/landlock/process_io.rb +++ b/lib/landlock/process_io.rb @@ -172,12 +172,7 @@ def wait_for_pid_until_by_polling(pid, deadline:) result = ::Process.wait2(pid, ::Process::WNOHANG) if result status = result.last - if monotonic_time >= deadline - terminate_process(pid) - return status, true - end - - return status, false + return status, monotonic_time >= deadline end IO.select(nil, nil, nil, [remaining, PID_WAIT_FALLBACK_INTERVAL_SECONDS].min) diff --git a/test/landlock_capture_test.rb b/test/landlock_capture_test.rb index 97e603e..9015275 100644 --- a/test/landlock_capture_test.rb +++ b/test/landlock_capture_test.rb @@ -165,13 +165,7 @@ def test_capture_rechecks_deadline_after_pidfd_becomes_readable IO.stub(:for_fd, for_fd) do IO.stub(:select, select) do result = - Landlock.capture( - ["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.15"], - rlimits: { - open_files: 64 - }, - timeout: 0.1 - ) + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.15"], rlimits: { open_files: 64 }, timeout: 0.1) end end @@ -179,32 +173,32 @@ def test_capture_rechecks_deadline_after_pidfd_becomes_readable assert_predicate result.status, :success? end - def test_capture_rechecks_deadline_before_accepting_polled_status + def test_capture_does_not_signal_reaped_pid_when_deadline_expires_during_poll skip "Landlock unsupported" unless Landlock.supported? pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) - original_select = IO.method(:select) - select = - lambda do |readers, writers = nil, errors = nil, timeout = nil| - if readers.nil? && writers.nil? && errors.nil? - sleep 1 - nil - else - original_select.call(readers, writers, errors, timeout) - end + original_wait2 = Process.method(:wait2) + original_kill = Process.method(:kill) + child_reaped = false + wait2 = + lambda do |*arguments| + sleep 0.15 if arguments.last == Process::WNOHANG + original_wait2.call(*arguments).tap { |result| child_reaped = true if result } + end + kill = + lambda do |*arguments| + flunk "capture signaled a reused PID after reaping the child" if child_reaped + + original_kill.call(*arguments) end result = nil Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do - IO.stub(:select, select) do - result = - Landlock.capture( - ["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.15"], - rlimits: { - open_files: 64 - }, - timeout: 0.1 - ) + Process.stub(:wait2, wait2) do + Process.stub(:kill, kill) do + result = + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; exit 0"], rlimits: { open_files: 64 }, timeout: 0.1) + end end end From 4866cae7956d09a351f4969308d9b4adef70e55e Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 20 Aug 2026 15:19:26 +0800 Subject: [PATCH 5/6] FIX: Kill late capture process groups safely A polling wait can reap the direct child after its deadline while same-group descendants remain alive. The existing generic termination path cannot be used safely because it may fall back to the now-recyclable positive PID. Send one immediate KILL only to the process group, retain the direct child status, and keep the result marked as timed out. Cover the race through the public capture API with a live descendant and reject positive-PID signals after reap. --- lib/landlock/process_io.rb | 13 ++++++- test/landlock_capture_test.rb | 71 +++++++++++++++++++++++------------ 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/lib/landlock/process_io.rb b/lib/landlock/process_io.rb index ddd1733..63b9d52 100644 --- a/lib/landlock/process_io.rb +++ b/lib/landlock/process_io.rb @@ -172,7 +172,12 @@ def wait_for_pid_until_by_polling(pid, deadline:) result = ::Process.wait2(pid, ::Process::WNOHANG) if result status = result.last - return status, monotonic_time >= deadline + if monotonic_time >= deadline + terminate_process_group(pid) + return status, true + end + + return status, false end IO.select(nil, nil, nil, [remaining, PID_WAIT_FALLBACK_INTERVAL_SECONDS].min) @@ -268,6 +273,12 @@ def terminate_process(pid) signal_process("KILL", pid) end + def terminate_process_group(pid) + ::Process.kill("KILL", -pid) + rescue Errno::ESRCH, Errno::EPERM + end + private_class_method :terminate_process_group + def signal_process(signal, pid) ::Process.kill(signal, -pid) rescue Errno::ESRCH, Errno::EPERM diff --git a/test/landlock_capture_test.rb b/test/landlock_capture_test.rb index 9015275..a42ab29 100644 --- a/test/landlock_capture_test.rb +++ b/test/landlock_capture_test.rb @@ -173,37 +173,60 @@ def test_capture_rechecks_deadline_after_pidfd_becomes_readable assert_predicate result.status, :success? end - def test_capture_does_not_signal_reaped_pid_when_deadline_expires_during_poll + def test_capture_fallback_kills_descendants_without_signaling_reaped_pid_when_deadline_expires_during_poll skip "Landlock unsupported" unless Landlock.supported? - pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) - original_wait2 = Process.method(:wait2) - original_kill = Process.method(:kill) - child_reaped = false - wait2 = - lambda do |*arguments| - sleep 0.15 if arguments.last == Process::WNOHANG - original_wait2.call(*arguments).tap { |result| child_reaped = true if result } - end - kill = - lambda do |*arguments| - flunk "capture signaled a reused PID after reaping the child" if child_reaped + Dir.mktmpdir do |dir| + pidfile = File.join(dir, "descendant.pid") + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) + original_wait2 = Process.method(:wait2) + original_kill = Process.method(:kill) + child_reaped = false + wait2 = + lambda do |*arguments| + sleep 0.15 if arguments.last == Process::WNOHANG + original_wait2.call(*arguments).tap { |result| child_reaped = true if result } + end + kill = + lambda do |signal, target| + flunk "capture signaled a reused PID after reaping the child" if child_reaped && target.positive? - original_kill.call(*arguments) - end + original_kill.call(signal, target) + end - result = nil - Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do - Process.stub(:wait2, wait2) do - Process.stub(:kill, kill) do - result = - Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; exit 0"], rlimits: { open_files: 64 }, timeout: 0.1) + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + Process.stub(:wait2, wait2) do + Process.stub(:kill, kill) do + result = + Landlock.capture( + [ + RbConfig.ruby, + "--disable=gems", + "-e", + "pid = Process.fork { STDOUT.close; STDERR.close; sleep 30 }; File.write(ARGV.fetch(0), pid); STDOUT.close; STDERR.close", + pidfile + ], + read: runtime_paths, + write: [dir], + execute: runtime_paths, + env: { + "PATH" => ENV.fetch("PATH", "") + }, + unsetenv_others: true, + timeout: 0.1 + ) + end end end - end - assert_predicate result, :timed_out? - assert_predicate result.status, :success? + assert_predicate result, :timed_out? + assert_predicate result.status, :success? + assert_path_exists pidfile + refute_process_alive Integer(File.read(pidfile)) + ensure + kill_process_from_file(pidfile) + end end def test_capture_does_not_create_timeout_thread_after_streams_close From 9c17c108aa69ea237ca8591c6c7936135326b2d9 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 20 Aug 2026 15:27:27 +0800 Subject: [PATCH 6/6] DEV: Keep capture polling fix unreleased Keep the gem at version 0.4 and record the capture polling fix under the Unreleased changelog section until the next release is prepared. --- CHANGELOG.md | 2 -- lib/landlock/version.rb | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f5b38b..3977156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,6 @@ All notable changes to this project will be documented in this file. ## Unreleased -## [0.4.1] - 2026-08-20 - ### Fixed - Remove the 100 ms child-exit polling interval from subprocess capture. Capture now drains stdout and stderr, then monitors the child via pidfd until exit or the wall-clock deadline, retaining polling only as a compatibility fallback when pidfds are unavailable. diff --git a/lib/landlock/version.rb b/lib/landlock/version.rb index 2b1ac44..1ec5bbf 100644 --- a/lib/landlock/version.rb +++ b/lib/landlock/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Landlock - VERSION = "0.4.1" + VERSION = "0.4" end