diff --git a/Cargo.lock b/Cargo.lock index 7c8e22c..3c90168 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -349,6 +349,15 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aa9a19cbb55df58761df49b23516a86d432839add4af60fc256da840f66ed35b" +[[package]] +name = "forkguard" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "723f03dfba28f1949cae68c6ac89a1e77f753c38c0acfceeefec77578a1c357a" +dependencies = [ + "libc", +] + [[package]] name = "form_urlencoded" version = "1.2.2" @@ -1547,6 +1556,7 @@ dependencies = [ "arc-swap", "bytes", "cookie", + "forkguard", "futures-util", "http", "http-body-util", diff --git a/Cargo.toml b/Cargo.toml index 02ce3f2..61a2ecf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,9 @@ http = "1.4.1" http-body-util = "0.1.3" futures-util = { version = "0.3.32", default-features = false } +[target.'cfg(unix)'.dependencies] +forkguard = { version = "0.1.3", features = ["atfork"] } + [dev-dependencies] magnus = { version = "0.8.2", features = ["embed"] } diff --git a/docs/fork-safety.md b/docs/fork-safety.md new file mode 100644 index 0000000..2824be3 --- /dev/null +++ b/docs/fork-safety.md @@ -0,0 +1,32 @@ +# Fork safety + +## Why inherited clients are rejected + +wreq-ruby uses a process-wide Tokio runtime and connection pool. `fork` copies +the parent's memory, but only the thread that called `fork` continues in the +child. Tokio's worker threads are gone, and its inherited tasks, locks, and +connections are not safe to reuse. + +If the parent has already loaded wreq-ruby, native HTTP operations in the child +raise `Wreq::ForkError`. This applies to new and existing clients, module +request methods, streaming request bodies, and response body methods. Retrying +the operation in the same child raises the same error. + +The parent can continue using its clients. When inherited Ruby objects are +collected in the child, their native runtime state is left for the operating +system to reclaim when the process exits. + +## Child processes are unsupported + +A process created with `fork` must not use wreq-ruby, even when it first loads +the extension after the fork. If the parent loaded wreq-ruby, native operations +in the child raise `Wreq::ForkError`. + +When the extension was not present in the parent, no wreq-ruby state or fork +marker reaches the child. The extension cannot reliably distinguish that child +from a newly started process, so this unsupported path cannot guarantee a Ruby +error and may fail inside platform libraries. + +Prefork servers should use an `exec`- or spawn-based worker model when workers +need wreq-ruby. Requiring the extension again does not reset inherited runtime +state, and there is no `after_fork!` hook. diff --git a/lib/wreq.rb b/lib/wreq.rb index 4f23bc9..18ccb36 100644 --- a/lib/wreq.rb +++ b/lib/wreq.rb @@ -28,6 +28,9 @@ module Wreq # raise ArgumentError. Known values retain the error class from their Ruby # or native conversion, such as TypeError or Wreq::BuilderError. Validation # finishes before network I/O. + # + # If a child process inherits wreq-ruby from its parent, requests raise + # Wreq::ForkError. Require wreq after the worker has been forked. # Send an HTTP request. # @@ -60,6 +63,7 @@ module Wreq # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.request(method, url, **options) end @@ -93,6 +97,7 @@ def self.request(method, url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.get(url, **options) end @@ -126,6 +131,7 @@ def self.get(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.head(url, **options) end @@ -159,6 +165,7 @@ def self.head(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.post(url, **options) end @@ -192,6 +199,7 @@ def self.post(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.put(url, **options) end @@ -225,6 +233,7 @@ def self.put(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.delete(url, **options) end @@ -258,6 +267,7 @@ def self.delete(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.options(url, **options) end @@ -291,6 +301,7 @@ def self.options(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.trace(url, **options) end @@ -324,6 +335,7 @@ def self.trace(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.patch(url, **options) end end diff --git a/lib/wreq_ruby/body.rb b/lib/wreq_ruby/body.rb index 549b517..1860d93 100644 --- a/lib/wreq_ruby/body.rb +++ b/lib/wreq_ruby/body.rb @@ -17,6 +17,8 @@ module Wreq # # A sender can be attached to one request. Closing it prevents further writes but # retains queued chunks so a request attached afterward can still drain them. + # Creating or using a sender raises Wreq::ForkError if the child inherited + # wreq-ruby from its parent. class BodySender # Create a bounded request-body sender. # @@ -25,6 +27,7 @@ class BodySender # @return [Wreq::BodySender] A streaming request body sender # @raise [ArgumentError] if capacity is zero, negative, or too large # @raise [TypeError] if capacity is not an Integer + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def self.new(capacity = 8) end @@ -33,6 +36,7 @@ def self.new(capacity = 8) # @param data [String] binary chunk # @return [nil] # @raise [IOError] if the sender or receiving side is closed + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def push(data) end @@ -41,6 +45,7 @@ def push(data) # This operation is idempotent. # # @return [nil] + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def close end @@ -50,6 +55,7 @@ def close # the receiving side. # # @return [Boolean] + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def closed? end end diff --git a/lib/wreq_ruby/client.rb b/lib/wreq_ruby/client.rb index c9bef7c..e2ff2a3 100644 --- a/lib/wreq_ruby/client.rb +++ b/lib/wreq_ruby/client.rb @@ -17,6 +17,10 @@ module Wreq # native conversion, such as TypeError or Wreq::BuilderError. Request # validation finishes before network I/O. # + # A child process cannot create or use a client if it inherited wreq-ruby + # from its parent. These calls raise Wreq::ForkError before accessing the + # native runtime. Require wreq after the worker has been forked. + # # @example Basic usage # client = Wreq::Client.new # # Use client for HTTP requests @@ -165,6 +169,7 @@ class Client # value cannot be converted or validated. # @raise [Wreq::BuilderError, Wreq::TlsError] if the native client cannot # be initialized. + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # # @example Minimal client # client = Wreq::Client.new @@ -280,6 +285,7 @@ def self.new(**options) # or unavailable on the current platform # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def request(method, url, **options) end @@ -313,6 +319,7 @@ def request(method, url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def get(url, **options) end @@ -346,6 +353,7 @@ def get(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def head(url, **options) end @@ -379,6 +387,7 @@ def head(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def post(url, **options) end @@ -412,6 +421,7 @@ def post(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def put(url, **options) end @@ -445,6 +455,7 @@ def put(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def delete(url, **options) end @@ -478,6 +489,7 @@ def delete(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def options(url, **options) end @@ -511,6 +523,7 @@ def options(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def trace(url, **options) end @@ -544,6 +557,7 @@ def trace(url, **options) # @return [Wreq::Response] HTTP response # @raise [TypeError, ArgumentError, Wreq::BuilderError] if a known option # value cannot be converted, validated, or built + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def patch(url, **options) end end diff --git a/lib/wreq_ruby/error.rb b/lib/wreq_ruby/error.rb index 65dd4dc..a7eb353 100644 --- a/lib/wreq_ruby/error.rb +++ b/lib/wreq_ruby/error.rb @@ -7,6 +7,19 @@ module Wreq # Memory allocation failed. class MemoryError < StandardError; end + # The child process inherited wreq-ruby from its parent. + # + # Tokio worker threads do not survive fork, and inherited pooled + # connections are not safe to reuse. This error is raised before a child + # can access that state. + # + # @example + # Process.fork do + # Wreq::Client.new # Raises if the parent loaded wreq-ruby. + # end + # @see https://github.com/SearchApi/wreq-ruby/blob/main/docs/fork-safety.md + class ForkError < RuntimeError; end + # Network connection errors # Connection to the server failed. diff --git a/lib/wreq_ruby/response.rb b/lib/wreq_ruby/response.rb index fc77eac..8a497db 100644 --- a/lib/wreq_ruby/response.rb +++ b/lib/wreq_ruby/response.rb @@ -8,6 +8,9 @@ module Wreq # access to HTTP response data including status codes, headers, body # content, and streaming capabilities. # + # Body methods raise Wreq::ForkError if the child inherited wreq-ruby from + # its parent. + # # @example Basic response handling # response = client.get("https://api.example.com") # puts response.status.as_int # => 200 @@ -107,6 +110,7 @@ def cookies # Get the response bytes as a binary string. # @return [String] Response body as binary data + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # @example # binary_data = response.bytes # puts binary_data.size # => 1024 @@ -122,6 +126,7 @@ def bytes # html = response.text("ISO-8859-1") # puts html # @raise [Wreq::DecodingError] if body cannot be decoded with the specified encoding + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def text(default_encoding = "UTF-8") end @@ -132,6 +137,7 @@ def text(default_encoding = "UTF-8") # # @return [Object] Parsed JSON (Hash, Array, String, Integer, Float, Boolean, nil) # @raise [Wreq::DecodingError] if body is not valid JSON + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # @example # data = response.json # puts data["key"] @@ -149,6 +155,7 @@ def json # @raise [LocalJumpError] if called without a block # @raise [Wreq::TimeoutError, Wreq::BodyError, Wreq::ConnectionResetError, Wreq::RequestError] # if streaming fails while reading the response body + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # @example Save response to file # File.open("output.bin", "wb") do |f| # response.chunks { |chunk| f.write(chunk) } @@ -165,6 +172,7 @@ def chunks # Close the response and free associated resources. # # @return [void] + # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # @example # response.close def close diff --git a/src/arch.rs b/src/arch.rs index cf427d5..91d542b 100644 --- a/src/arch.rs +++ b/src/arch.rs @@ -6,6 +6,49 @@ //! do not leak into the rest of the binding. #![allow(unsafe_code)] +use std::mem::ManuallyDrop; + +/// Native state that belongs to the process where the extension was loaded. +/// +/// A forked child must not destroy inherited clients, channels, or response +/// bodies because their synchronization state may belong to threads that no +/// longer exist. The child intentionally leaks the value and lets the operating +/// system reclaim it when the process exits. +/// +/// This wrapper only controls destruction. Call [`crate::rt::ensure_current`] +/// before accessing the inner value. +#[derive(Clone)] +pub(crate) struct ProcessLocal(ManuallyDrop); + +impl ProcessLocal { + /// Wrap native state created by the current process. + pub(crate) fn new(value: T) -> Self { + Self(ManuallyDrop::new(value)) + } +} + +impl AsRef for ProcessLocal { + fn as_ref(&self) -> &T { + &self.0 + } +} + +impl Drop for ProcessLocal { + fn drop(&mut self) { + #[cfg(unix)] + if forked_process_ids().is_some() { + return; + } + + // SAFETY: `new` initializes the value exactly once, `ManuallyDrop` + // prevents an automatic second drop, and this wrapper's `Drop` + // implementation runs at most once. + unsafe { + ManuallyDrop::drop(&mut self.0); + } + } +} + /// Whether the native client exposes TCP user-timeout configuration. pub(crate) const SUPPORTS_TCP_USER_TIMEOUT: bool = cfg!(any( target_os = "android", @@ -27,6 +70,64 @@ pub(crate) const SUPPORTS_INTERFACE: bool = cfg!(any( target_os = "watchos", )); +#[cfg(unix)] +mod unix { + use std::{io, process, sync::OnceLock}; + + /// Process state captured when the extension initializes. + /// + /// The atfork guard uses a POSIX child handler to advance an atomic fork + /// generation without running Ruby code. + /// https://pubs.opengroup.org/onlinepubs/9799919799/functions/pthread_atfork.html + struct ForkGuard { + detector: forkguard::Guard, + owner_pid: u32, + } + + impl ForkGuard { + /// Create a guard and register fork detection with the process. + fn new() -> io::Result { + forkguard::Guard::try_new() + .map(|detector| Self { + detector, + owner_pid: process::id(), + }) + .map_err(|error| io::Error::from_raw_os_error(error.code().get())) + } + + /// Return process IDs when this guard was inherited through a fork. + fn forked_process_ids(&self) -> Option<(u32, u32)> { + // Keep the stored generation unchanged so every runtime access in + // the child remains rejected. Cloning the detector copies one usize. + let mut detector = self.detector.clone(); + detector + .detected_fork() + .then(|| (self.owner_pid, process::id())) + } + } + + static FORK_GUARD: OnceLock = OnceLock::new(); + + /// Register process fork tracking before the extension exposes its API. + pub(crate) fn initialize_fork_tracking() -> io::Result<()> { + if FORK_GUARD.get().is_some() { + return Ok(()); + } + + let guard = ForkGuard::new()?; + let _ = FORK_GUARD.set(guard); + Ok(()) + } + + /// Return process IDs only when this process inherited the extension. + pub(crate) fn forked_process_ids() -> Option<(u32, u32)> { + FORK_GUARD.get().and_then(ForkGuard::forked_process_ids) + } +} + +#[cfg(unix)] +pub(crate) use unix::{forked_process_ids, initialize_fork_tracking}; + #[cfg(all(target_os = "windows", target_env = "gnu"))] mod windows_gnu { //! Windows GNU support. @@ -53,3 +154,30 @@ mod windows_gnu { } } } + +#[cfg(test)] +mod tests { + use std::cell::Cell; + + use super::ProcessLocal; + + struct DropCounter<'a>(&'a Cell); + + impl Drop for DropCounter<'_> { + fn drop(&mut self) { + self.0.set(self.0.get() + 1); + } + } + + #[test] + fn process_local_drops_in_its_owner_process() { + let drops = Cell::new(0); + + { + let value = ProcessLocal::new(DropCounter(&drops)); + assert_eq!(value.as_ref().0.get(), 0); + } + + assert_eq!(drops.get(), 1); + } +} diff --git a/src/client.rs b/src/client.rs index 3bdec58..6f8bf85 100644 --- a/src/client.rs +++ b/src/client.rs @@ -11,7 +11,7 @@ use magnus::{Module, Object, RModule, Ruby, TryConvert, Value, function, method, use wreq::Proxy; use crate::{ - arch::{SUPPORTS_INTERFACE, SUPPORTS_TCP_USER_TIMEOUT}, + arch::{ProcessLocal, SUPPORTS_INTERFACE, SUPPORTS_TCP_USER_TIMEOUT}, client::{req::execute_request, resp::Response}, cookie::Jar, emulate::Emulation, @@ -21,6 +21,7 @@ use crate::{ header::{Headers, OrigHeaders, UserAgent}, http::Method, options::{NativeOption, Options}, + rt, }; /// A builder for `Client`. @@ -120,7 +121,7 @@ struct Builder { #[derive(Clone)] #[magnus::wrap(class = "Wreq::Client", free_immediately, size)] -pub struct Client(wreq::Client); +pub struct Client(ProcessLocal); // ===== impl Builder ===== @@ -193,11 +194,15 @@ impl Client { /// native fallible client builder. Extra positional arguments return /// `ArgumentError`. pub fn new(ruby: &Ruby, args: &[Value]) -> Result { + rt::ensure_current(ruby)?; + Options::from_args(ruby, args, "client")? .map(Builder::from_options) .transpose() .map(Option::unwrap_or_default) .and_then(|params| Self::build(ruby, params)) + .map(ProcessLocal::new) + .map(Self) } /// Build the default client through the same fallible path as `new`. @@ -206,7 +211,7 @@ impl Client { /// /// Returns `Wreq::BuilderError`, `Wreq::TlsError`, or another mapped native /// initialization error without unwinding through Ruby. - pub(crate) fn default_client(ruby: &Ruby) -> Result { + pub(crate) fn default_client(ruby: &Ruby) -> Result { Self::build(ruby, Builder::default()) } @@ -214,8 +219,12 @@ impl Client { /// /// # Errors /// - /// Maps native build failures only after the GVL has been reacquired. - fn build(ruby: &Ruby, mut params: Builder) -> Result { + /// Returns `Wreq::ForkError` before touching native client state when the + /// extension was inherited from a parent process. Maps native build + /// failures only after the GVL has been reacquired. + fn build(ruby: &Ruby, mut params: Builder) -> Result { + rt::ensure_current(ruby)?; + let result = gvl::nogvl(|| { let mut builder = wreq::Client::builder(); @@ -374,12 +383,17 @@ impl Client { apply_option!(set_if_some, builder, params.deflate, deflate); apply_option!(set_if_some, builder, params.zstd, zstd); - builder.build().map(Client) + builder.build() }); // Ruby exceptions must be created after the GVL has been reacquired. result.map_err(|err| wreq_error(ruby, err)) } + + /// Clone the native client handle for a request future. + fn native_client(&self) -> wreq::Client { + self.0.as_ref().clone() + } } impl Client { @@ -391,9 +405,9 @@ impl Client { ruby: &Ruby, args: &[Value], ) -> Result { - let ((method, url), request) = extract_request!(args, (Obj, String)); + let ((method, url), request) = extract_request!(ruby, args, (Obj, String)); let client = Self::default_client(ruby)?; - execute_request(ruby, client.0, *method, url, request) + execute_request(ruby, client, *method, url, request) } /// Send a request with `method` through a newly built default client. @@ -405,72 +419,72 @@ impl Client { method: Method, args: &[Value], ) -> Result { - let ((url,), request) = extract_request!(args, (String,)); + let ((url,), request) = extract_request!(ruby, args, (String,)); let client = Self::default_client(ruby)?; - execute_request(ruby, client.0, method, url, request) + execute_request(ruby, client, method, url, request) } /// Send a HTTP request. #[inline] pub fn request(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((method, url), request) = extract_request!(args, (Obj, String)); - execute_request(ruby, rb_self.0.clone(), *method, url, request) + let ((method, url), request) = extract_request!(ruby, args, (Obj, String)); + execute_request(ruby, rb_self.native_client(), *method, url, request) } /// Send a GET request. #[inline] pub fn get(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::GET, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::GET, url, request) } /// Send a POST request. #[inline] pub fn post(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::POST, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::POST, url, request) } /// Send a PUT request. #[inline] pub fn put(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::PUT, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::PUT, url, request) } /// Send a DELETE request. #[inline] pub fn delete(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::DELETE, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::DELETE, url, request) } /// Send a HEAD request. #[inline] pub fn head(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::HEAD, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::HEAD, url, request) } /// Send an OPTIONS request. #[inline] pub fn options(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::OPTIONS, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::OPTIONS, url, request) } /// Send a TRACE request. #[inline] pub fn trace(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::TRACE, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::TRACE, url, request) } /// Send a PATCH request. #[inline] pub fn patch(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { - let ((url,), request) = extract_request!(args, (String,)); - execute_request(ruby, rb_self.0.clone(), Method::PATCH, url, request) + let ((url,), request) = extract_request!(ruby, args, (String,)); + execute_request(ruby, rb_self.native_client(), Method::PATCH, url, request) } } diff --git a/src/client/body/stream.rs b/src/client/body/stream.rs index 88c194b..5a210d0 100644 --- a/src/client/body/stream.rs +++ b/src/client/body/stream.rs @@ -13,6 +13,7 @@ use magnus::{Error, Integer, RString, Ruby, Value, scan_args::scan_args}; use tokio::sync::{Mutex, Semaphore, mpsc}; use crate::{ + arch::ProcessLocal, error::{ argument_error, body_sender_borrow_error, body_sender_borrow_mut_error, body_sender_send_error, closed_body_sender_error, memory_error, type_error, wreq_error, @@ -33,7 +34,7 @@ pub struct BodyReceiver(Mutex> + S /// receiver. Ruby's GVL protects state access; no [`RefCell`] borrow is kept /// while request backpressure waits without the GVL. #[magnus::wrap(class = "Wreq::BodySender", free_immediately, size)] -pub struct BodySender(RefCell); +pub struct BodySender(ProcessLocal>); /// Mutable ownership state for both halves of the body channel. struct InnerBodySender { @@ -89,8 +90,10 @@ impl BodySender { /// # Errors /// /// Returns `TypeError` for a non-Integer capacity and `ArgumentError` for - /// an invalid range or argument count. + /// an invalid range or argument count. Returns `Wreq::ForkError` before + /// creating a channel in a child that inherited the extension. pub fn new(ruby: &Ruby, args: &[Value]) -> Result { + rt::ensure_current(ruby)?; let capacity = parse_capacity(ruby, args)?; // Create the Tokio channel without allowing an unwind to cross the Ruby FFI boundary. @@ -100,10 +103,12 @@ impl BodySender { let (tx, rx) = catch_unwind(|| mpsc::channel(capacity)).map_err(|_| invalid_capacity_error(ruby))?; - Ok(BodySender(RefCell::new(InnerBodySender { - tx: Some(tx), - rx: Some(rx), - }))) + Ok(BodySender(ProcessLocal::new(RefCell::new( + InnerBodySender { + tx: Some(tx), + rx: Some(rx), + }, + )))) } /// Push a binary chunk, waiting for capacity when the channel is full. @@ -113,8 +118,11 @@ impl BodySender { /// # Errors /// /// Returns `IOError` after either channel side has closed. An interrupted - /// wait raises Ruby's standard `Interrupt` exception. + /// wait raises Ruby's standard `Interrupt` exception. Returns + /// `Wreq::ForkError` before reading an inherited channel. pub fn push(ruby: &Ruby, rb_self: &Self, data: RString) -> Result<(), Error> { + rt::ensure_current(ruby)?; + // Clone during the shared borrow, then release it before waiting // for capacity. Request attachment needs a mutable borrow. let tx = match &rb_self.read_inner(ruby)?.tx { @@ -131,8 +139,10 @@ impl BodySender { /// /// # Errors /// - /// Returns `Wreq::BodyError` if the internal state is already borrowed. + /// Returns `Wreq::ForkError` before reading an inherited channel, or + /// `Wreq::BodyError` if the internal state is already borrowed. pub fn close(ruby: &Ruby, rb_self: &Self) -> Result<(), Error> { + rt::ensure_current(ruby)?; let mut inner = rb_self.write_inner(ruby)?; inner.tx.take(); Ok(()) @@ -142,14 +152,17 @@ impl BodySender { /// /// # Errors /// - /// Returns `Wreq::BodyError` if the internal state is already borrowed. + /// Returns `Wreq::ForkError` before reading an inherited channel, or + /// `Wreq::BodyError` if the internal state is already borrowed. pub fn is_closed(ruby: &Ruby, rb_self: &Self) -> Result { + rt::ensure_current(ruby)?; rb_self.read_inner(ruby).map(|r| r.is_closed()) } /// Borrow the channel state without panicking on accidental re-entry. fn read_inner(&self, ruby: &Ruby) -> Result, Error> { self.0 + .as_ref() .try_borrow() .map_err(|err| body_sender_borrow_error(ruby, err)) } @@ -157,6 +170,7 @@ impl BodySender { /// Mutably borrow the channel state without panicking on accidental re-entry. fn write_inner(&self, ruby: &Ruby) -> Result, Error> { self.0 + .as_ref() .try_borrow_mut() .map_err(|err| body_sender_borrow_mut_error(ruby, err)) } @@ -168,6 +182,7 @@ impl BodySender { /// Returns `Wreq::MemoryError` if the receiver was already consumed, or /// `Wreq::BodyError` if Ruby re-enters while the state is borrowed. pub(super) fn take_receiver(&self, ruby: &Ruby) -> Result, Error> { + rt::ensure_current(ruby)?; self.write_inner(ruby)? .rx .take() diff --git a/src/client/resp.rs b/src/client/resp.rs index 9fbc08c..9cc2f98 100644 --- a/src/client/resp.rs +++ b/src/client/resp.rs @@ -9,6 +9,7 @@ use magnus::{Error, Module, RArray, RModule, Ruby, Value, scan_args::scan_args}; use wreq::Uri; use crate::{ + arch::ProcessLocal, client::body::{json::Json, stream::BodyReceiver}, cookie::Cookie, error::{memory_error, no_block_given_error, wreq_error}, @@ -28,8 +29,7 @@ pub struct Response { headers: HeaderMap, local_addr: Option, remote_addr: Option, - body: ArcSwapOption, - extensions: Extensions, + state: ProcessLocal, } /// Represents the state of the HTTP response body. @@ -40,6 +40,12 @@ enum Body { Reusable(Bytes), } +/// Response state that may contain handles owned by the native runtime. +struct NativeResponseState { + body: ArcSwapOption, + extensions: Extensions, +} + impl Response { /// Create a new [`Response`] instance. pub fn new(response: wreq::Response) -> Self { @@ -55,26 +61,31 @@ impl Response { local_addr, remote_addr, content_length, - extensions: parts.extensions, version: Version::from_ffi(parts.version), status: StatusCode::from(parts.status), headers: parts.headers, - body: ArcSwapOption::from_pointee(Body::Streamable(body)), + state: ProcessLocal::new(NativeResponseState { + body: ArcSwapOption::from_pointee(Body::Streamable(body)), + extensions: parts.extensions, + }), } } /// Internal method to get the wreq::Response, optionally streaming the body. fn response(&self, ruby: &Ruby, stream: bool) -> Result { + rt::ensure_current(ruby)?; + let state = self.state.as_ref(); + let build_response = |body: wreq::Body| -> wreq::Response { let mut response = HttpResponse::new(body); *response.version_mut() = self.version.into_ffi(); *response.status_mut() = self.status.0; *response.headers_mut() = self.headers.clone(); - *response.extensions_mut() = self.extensions.clone(); + *response.extensions_mut() = state.extensions.clone(); wreq::Response::from(response) }; - if let Some(arc) = self.body.swap(None) { + if let Some(arc) = state.body.swap(None) { match Arc::try_unwrap(arc) { Ok(Body::Streamable(body)) => { return if stream { @@ -86,14 +97,16 @@ impl Response { wreq_error, )?; - self.body + state + .body .store(Some(Arc::new(Body::Reusable(bytes.clone())))); Ok(build_response(wreq::Body::from(bytes))) }; } Ok(Body::Reusable(bytes)) => { - self.body + state + .body .store(Some(Arc::new(Body::Reusable(bytes.clone())))); if !stream { @@ -175,6 +188,7 @@ impl Response { /// Get the full response text given a specific encoding. pub fn text(ruby: &Ruby, rb_self: &Self, args: &[Value]) -> Result { + rt::ensure_current(ruby)?; let args = scan_args::<(), (Option,), (), (), (), ()>(args)?; let response = rb_self.response(ruby, false)?; match args.optional.0 { @@ -194,6 +208,8 @@ impl Response { /// Yield response body chunks to the given Ruby block. pub fn chunks(ruby: &Ruby, rb_self: &Self) -> Result<(), Error> { + rt::ensure_current(ruby)?; + if !ruby.block_given() { return Err(no_block_given_error(ruby)); } @@ -211,16 +227,15 @@ impl Response { } /// Close the response body, dropping any resources. - #[inline] - pub fn close(&self) { - gvl::nogvl(|| self.body.swap(None)); - } -} - -impl Drop for Response { - fn drop(&mut self) { - // Ensure body is dropped in GVL - self.body.swap(None); + /// + /// # Errors + /// + /// Returns `Wreq::ForkError` before touching a body inherited from the + /// parent process. + pub fn close(ruby: &Ruby, rb_self: &Self) -> Result<(), Error> { + rt::ensure_current(ruby)?; + gvl::nogvl(|| rb_self.state.as_ref().body.swap(None)); + Ok(()) } } diff --git a/src/error.rs b/src/error.rs index 15fa60e..5de9fbd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -56,6 +56,7 @@ macro_rules! map_wreq_error { // System-level and runtime errors define_exception!(MEMORY, "MemoryError", exception_runtime_error); +define_exception!(FORK_ERROR, "ForkError", exception_runtime_error); // Network connection errors define_exception!(CONNECTION_ERROR, "ConnectionError", exception_runtime_error); @@ -94,6 +95,26 @@ pub fn interrupt_error(ruby: &Ruby) -> MagnusError { MagnusError::new(ruby.exception_interrupt(), "request interrupted") } +/// Build `Wreq::ForkError` without touching inherited native state. +#[cfg(unix)] +pub fn fork_error(ruby: &Ruby, owner_pid: u32, current_pid: u32) -> MagnusError { + MagnusError::new( + ruby.get_inner(&FORK_ERROR), + format!( + "wreq-ruby was loaded in process {owner_pid} and cannot be used after fork in process {current_pid}" + ), + ) +} + +/// Map a failed process-fork handler registration to `Wreq::ForkError`. +#[cfg(unix)] +pub fn fork_handler_error(ruby: &Ruby, err: &std::io::Error) -> MagnusError { + MagnusError::new( + ruby.get_inner(&FORK_ERROR), + format!("failed to initialize process fork tracking: {err}"), + ) +} + /// Map a Tokio runtime initialization failure to `Wreq::BuilderError`. pub fn runtime_initialization_error(ruby: &Ruby, err: &std::io::Error) -> MagnusError { MagnusError::new( @@ -235,6 +256,13 @@ pub fn include(ruby: &Ruby, gem_module: &RModule) -> Result<(), MagnusError> { "MemoryError", exception_runtime_error ); + initialize_exception!( + ruby, + gem_module, + FORK_ERROR, + "ForkError", + exception_runtime_error + ); initialize_exception!( ruby, gem_module, diff --git a/src/lib.rs b/src/lib.rs index ae53f87..82d852d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -85,6 +85,7 @@ pub fn patch(ruby: &Ruby, args: &[Value]) -> Result { fn init(ruby: &Ruby) -> Result<(), Error> { let gem_module = ruby.define_module(RUBY_MODULE_NAME)?; gem_module.const_set("VERSION", VERSION)?; + error::include(ruby, &gem_module)?; gem_module.define_module_function("request", magnus::function!(request, -1))?; gem_module.define_module_function("get", magnus::function!(get, -1))?; gem_module.define_module_function("post", magnus::function!(post, -1))?; @@ -99,6 +100,7 @@ fn init(ruby: &Ruby) -> Result<(), Error> { cookie::include(ruby, &gem_module)?; client::include(ruby, &gem_module)?; emulate::include(ruby, &gem_module)?; - error::include(ruby, &gem_module)?; + #[cfg(unix)] + rt::initialize(ruby)?; Ok(()) } diff --git a/src/macros.rs b/src/macros.rs index 02d3f5d..412318b 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -148,11 +148,11 @@ macro_rules! define_ruby_enum { } macro_rules! extract_request { - ($args:expr, $required:ty) => {{ + ($ruby:expr, $args:expr, $required:ty) => {{ + crate::rt::ensure_current($ruby)?; let args = magnus::scan_args::scan_args::<$required, (), (), (), magnus::RHash, ()>($args)?; let required = args.required; - let ruby = magnus::Ruby::get_with(args.keywords); - let request = crate::client::req::Request::new(&ruby, args.keywords)?; + let request = crate::client::req::Request::new($ruby, args.keywords)?; (required, request) }}; } diff --git a/src/rt.rs b/src/rt.rs index 28feba9..c037401 100644 --- a/src/rt.rs +++ b/src/rt.rs @@ -1,26 +1,57 @@ -use std::sync::LazyLock; +use std::{io, sync::OnceLock}; use magnus::Ruby; -use tokio::runtime::{Builder, Runtime}; +use tokio::runtime::{Builder, Runtime as TokioRuntime}; use crate::{ error::{interrupt_error, runtime_initialization_error}, gvl, }; -/// Initialize the global runtime lazily and preserve failures for Ruby. -static RUNTIME: LazyLock> = LazyLock::new(|| { - let mut builder = Builder::new_multi_thread(); +#[cfg(unix)] +use crate::{ + arch, + error::{fork_error, fork_handler_error}, +}; - builder.enable_all().build() -}); +/// Initialize the global runtime lazily and preserve failures for Ruby. +static RUNTIME: OnceLock> = OnceLock::new(); enum BlockOnError { Interrupted, Future(E), } -/// Block on a future to completion on the global Tokio runtime. +/// Register fork tracking while the native extension is being loaded. +/// +/// # Errors +/// +/// Returns `Wreq::ForkError` if the platform cannot install its child-process +/// callback. +#[cfg(unix)] +pub fn initialize(ruby: &Ruby) -> Result<(), magnus::Error> { + arch::initialize_fork_tracking().map_err(|err| fork_handler_error(ruby, &err)) +} + +/// Reject a child process that inherited the loaded native extension. +/// +/// # Errors +/// +/// Returns `Wreq::ForkError` when the extension was loaded before the current +/// process was forked. +pub fn ensure_current(ruby: &Ruby) -> Result<(), magnus::Error> { + #[cfg(unix)] + if let Some((owner_pid, current_pid)) = arch::forked_process_ids() { + return Err(fork_error(ruby, owner_pid, current_pid)); + } + + #[cfg(not(unix))] + let _ = ruby; + + Ok(()) +} + +/// Block on a future to completion on the current process's global Tokio runtime. /// /// The future runs without Ruby's GVL, so it must not construct Ruby objects or /// Ruby exceptions. Convert Rust errors back into Ruby errors after the GVL has @@ -28,15 +59,21 @@ enum BlockOnError { /// /// # Errors /// -/// Returns `Wreq::BuilderError` if the Tokio runtime cannot be initialized, -/// Ruby's standard `Interrupt` if Ruby interrupts the request, or the error produced +/// Returns `Wreq::ForkError` if the extension belongs to a parent process, +/// `Wreq::BuilderError` if the Tokio runtime cannot be initialized, Ruby's +/// standard `Interrupt` if Ruby interrupts the request, or the error produced /// by `map_err` if the future fails. pub fn try_block_on(ruby: &Ruby, future: F, map_err: M) -> Result where F: Future>, M: FnOnce(&Ruby, E) -> magnus::Error, { + ensure_current(ruby)?; let runtime = RUNTIME + .get_or_init(|| { + let mut builder = Builder::new_multi_thread(); + builder.enable_all().build() + }) .as_ref() .map_err(|err| runtime_initialization_error(ruby, err))?; let result = gvl::nogvl_cancellable(|flag| { diff --git a/test/fork_test.rb b/test/fork_test.rb new file mode 100644 index 0000000..7fc7b1d --- /dev/null +++ b/test/fork_test.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require "test_helper" +require "rbconfig" +require "tempfile" +require "timeout" + +class ForkTest < Minitest::Test + FORK_ERROR_LABELS = %w[ + before_runtime + invalid_client + invalid_request + fresh_body_sender + inherited_body_sender_push + inherited_body_sender_close + inherited_body_sender_closed + fresh_client + inherited_client + inherited_response + inherited_response_text + inherited_response_chunks + inherited_response_close + ].freeze + + def test_fork_error_is_a_runtime_error + assert_operator Wreq::ForkError, :<, RuntimeError + end + + def test_loaded_extension_is_rejected_after_fork + skip "fork is not supported on this platform" unless Process.respond_to?(:fork) + + stdout, stderr, status = run_fork_script("fork_safety.rb") + + assert status.success?, "subprocess failed with #{status.inspect}: #{stderr}" + assert_equal "ok\n", stdout + FORK_ERROR_LABELS.each do |label| + assert_match(/#{label}=Wreq::ForkError:.*cannot be used after fork/, stderr) + assert_match(/#{label}_retry=Wreq::ForkError:.*cannot be used after fork/, stderr) + end + assert_match(/inherited_gc=ok/, stderr) + refute_match(/\[BUG\]|segmentation fault|panicked/i, stderr) + end + + private + + def run_fork_script(name) + lib_dir = File.expand_path("../lib", __dir__) + script = File.expand_path("scripts/#{name}", __dir__) + + Tempfile.create("wreq-fork-stdout") do |stdout| + Tempfile.create("wreq-fork-stderr") do |stderr| + pid = Process.spawn( + RbConfig.ruby, + "-I", + lib_dir, + script, + out: stdout, + err: stderr, + pgroup: true + ) + status = Timeout.timeout(30) { Process.wait2(pid).last } + stdout.rewind + stderr.rewind + return [stdout.read, stderr.read, status] + rescue Timeout::Error + begin + Process.kill("KILL", -pid) + rescue Errno::ESRCH + nil + end + + begin + Process.wait(pid) + rescue Errno::ECHILD + nil + end + + flunk "#{name} timed out" + end + end + end +end diff --git a/test/scripts/fork_safety.rb b/test/scripts/fork_safety.rb new file mode 100644 index 0000000..7593b40 --- /dev/null +++ b/test/scripts/fork_safety.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +require "socket" +require "timeout" +require "weakref" +require "wreq" + +$stdout.sync = true +$stderr.sync = true + +def expect_fork_error(label) + child_pid = fork do + 2.times do |attempt| + attempt_label = attempt.zero? ? label : "#{label}_retry" + + begin + Timeout.timeout(5) { yield } + rescue Wreq::ForkError => error + warn "#{attempt_label}=#{error.class}: #{error.message}" + next + rescue => error + warn "#{attempt_label}=unexpected #{error.class}: #{error.message}" + exit! 2 + end + + warn "#{attempt_label}=missing Wreq::ForkError" + exit! 3 + end + + exit! 0 + end + + _, status = Process.wait2(child_pid) + abort "#{label} child failed with #{status.inspect}" unless status.success? +end + +expect_fork_error("before_runtime") { Wreq::Client.new } +expect_fork_error("invalid_client") { Wreq::Client.new(unknown: true) } +expect_fork_error("invalid_request") { Wreq.get(1) } +expect_fork_error("fresh_body_sender") { Wreq::BodySender.new(0) } + +server = TCPServer.new("127.0.0.1", 0) +port = server.addr[1] +server_pid = fork do + 3.times do + ready = IO.select([server], nil, nil, 10) + exit! 4 unless ready + + socket = server.accept + begin + while (line = socket.gets) + break if line == "\r\n" + end + socket.write("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok") + ensure + socket.close + end + end + exit! 0 +ensure + server.close +end +server.close + +url = "http://127.0.0.1:#{port}/" +client = Wreq::Client.new +abort "parent warm-up failed" unless client.get(url).bytes == "ok" + +def build_inherited_objects(client, url) + [Wreq::Client.new, Wreq::BodySender.new, client.get(url)] +end + +inherited_objects = build_inherited_objects(client, url) +inherited_weak_refs = inherited_objects.map { |object| WeakRef.new(object) } + +expect_fork_error("inherited_body_sender_push") do + inherited_objects[1].push("chunk") +end +expect_fork_error("inherited_body_sender_close") { inherited_objects[1].close } +expect_fork_error("inherited_body_sender_closed") { inherited_objects[1].closed? } +expect_fork_error("fresh_client") { Wreq::Client.new } +expect_fork_error("inherited_client") { client.get(url) } +expect_fork_error("inherited_response") { inherited_objects[2].bytes } +expect_fork_error("inherited_response_text") { inherited_objects[2].text(1) } +expect_fork_error("inherited_response_chunks") { inherited_objects[2].chunks } +expect_fork_error("inherited_response_close") { inherited_objects[2].close } + +# Release the earlier test blocks so this array is the only strong reference. +GC.start(full_mark: true, immediate_sweep: true) +gc_pid = fork do + inherited_objects = nil + 3.times { GC.start(full_mark: true, immediate_sweep: true) } + alive = inherited_weak_refs.each_index.select do |index| + inherited_weak_refs[index].weakref_alive? + end + abort "inherited objects were not collected: #{alive.join(", ")}" unless alive.empty? + warn "inherited_gc=ok" + exit! 0 +rescue => error + warn "inherited_gc=unexpected #{error.class}: #{error.message}" + exit! 5 +end +_, gc_status = Process.wait2(gc_pid) +abort "inherited GC child failed with #{gc_status.inspect}" unless gc_status.success? + +abort "parent request after fork failed" unless client.get(url).bytes == "ok" +_, server_status = Process.wait2(server_pid) +abort "server failed with #{server_status.inspect}" unless server_status.success? + +puts "ok"