diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a80875e..5c0823f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,14 +34,13 @@ jobs: - name: Run cargo clippy run: cargo clippy --all-targets --all-features -- -D warnings - ruby-binding-tests: - name: Ruby Binding Tests + linux-tests: + name: Linux Tests (${{ matrix.os }}, ${{ matrix.ruby }}) runs-on: ${{ matrix.os }} env: HTTPBIN_URL: https://httpbin.io strategy: fail-fast: false - max-parallel: 1 matrix: os: [ubuntu-latest] ruby: ['3.3', '3.4', '4.0'] @@ -68,3 +67,32 @@ jobs: - name: Run tests run: bundle exec rake test + + windows-gnu-tests: + name: Ruby Binding Tests (windows-latest, ${{ matrix.ruby }}) + runs-on: windows-latest + env: + CARGO_BUILD_TARGET: x86_64-pc-windows-gnu + HTTPBIN_URL: https://httpbin.io + strategy: + fail-fast: false + matrix: + ruby: ['3.3', '3.4', '4.0'] + + steps: + - uses: actions/checkout@v6 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + with: + targets: x86_64-pc-windows-gnu + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + + - name: Run tests + shell: pwsh + run: ./script/build_windows_gnu.ps1 -SkipBundleInstall -RunTests diff --git a/build.rs b/build.rs index da1ad3c..ba4b10b 100644 --- a/build.rs +++ b/build.rs @@ -5,5 +5,12 @@ fn main() -> Result<(), Box> { // This is not a requirement, but it is a convenient if you want to use // `cargo test`, etc. let _ = rb_sys_env::activate()?; + + if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows") + && std::env::var("CARGO_CFG_TARGET_ENV").as_deref() == Ok("gnu") + { + println!("cargo:rustc-link-arg-cdylib=-Wl,--wrap=Sleep"); + } + Ok(()) } diff --git a/docs/windows-gnu-tokio-crash.md b/docs/windows-gnu-tokio-crash.md new file mode 100644 index 0000000..bc16a96 --- /dev/null +++ b/docs/windows-gnu-tokio-crash.md @@ -0,0 +1,70 @@ +# Windows GNU Tokio multi-thread crash + +## Summary + +On RubyInstaller GNU/UCRT, Tokio's multi-thread runtime could crash when the +native extension was loaded by Ruby. The issue was not Tokio's scheduler logic +itself. It came from Windows symbol resolution while linking the Ruby extension. + +## Root cause + +RubyInstaller GNU/UCRT exports a Ruby-aware `Sleep` symbol from +`x64-ucrt-ruby400.dll`. + +When the extension is linked with GNU ld, an unqualified `Sleep` reference can +bind to Ruby's `Sleep` instead of `KERNEL32!Sleep`. Tokio's multi-thread runtime +creates native worker threads that are not Ruby-managed threads. If one of those +threads calls Ruby's `Sleep`, Ruby expects Ruby thread-local state to exist, but +that TLS slot is empty on the Tokio worker thread. This can crash with an access +violation or Ruby VM bug report. + +The local investigation found the fault inside: + +```text +x64-ucrt-ruby400.dll!Sleep + 0x40 +``` + +The import table also showed the bad binding: + +```text +DLL Name: x64-ucrt-ruby400.dll + Sleep +``` + +## Fix + +For `x86_64-pc-windows-gnu`, the build now wraps `Sleep` at link time: + +```text +-Wl,--wrap=Sleep +``` + +The wrapper lives in `src/arch.rs` and forwards calls to `KERNEL32!SleepEx`. +This keeps Tokio's worker threads on the real Windows API path while preserving +Tokio's multi-thread runtime and Ruby GVL release behavior. + +After the fix, the import table shows: + +```text +DLL Name: KERNEL32.dll + Sleep + SleepEx +``` + +## Notes + +`windows-sys` raw-dylib was also tested. It is not enough on its own because the +problematic `Sleep` reference can come from Rust std, MinGW, or pthread-related +linking paths, not only from `windows-sys`. + +Verified with: + +```powershell +.\script\build_windows_gnu.ps1 -SkipBundleInstall -SkipToolInstall -RunTests +``` + +Result: + +```text +160 runs, 775 assertions, 0 failures, 0 errors +``` diff --git a/script/build_windows_gnu.ps1 b/script/build_windows_gnu.ps1 new file mode 100644 index 0000000..514f39e --- /dev/null +++ b/script/build_windows_gnu.ps1 @@ -0,0 +1,257 @@ +param( + [switch]$SkipBundleInstall, + [switch]$SkipToolInstall, + [switch]$BuildGem, + [switch]$TestGem, + [switch]$RunTests, + [string]$SmokeUrl = $env:WREQ_SMOKE_URL +) + +$ErrorActionPreference = "Stop" + +$target = "x86_64-pc-windows-gnu" +$platform = "x64-mingw-ucrt" +$gemPattern = "pkg\wreq-*-$platform.gem" +if ([string]::IsNullOrWhiteSpace($SmokeUrl)) { + $SmokeUrl = "https://httpbin.io/get" +} +$rubyRoot = Split-Path (Split-Path (Get-Command ruby).Source -Parent) -Parent +$ucrt = Join-Path $rubyRoot "msys64\ucrt64" +$ucrtBin = Join-Path $ucrt "bin" +$msysPackages = @( + "mingw-w64-ucrt-x86_64-gcc", + "mingw-w64-ucrt-x86_64-clang", + "mingw-w64-ucrt-x86_64-cmake", + "mingw-w64-ucrt-x86_64-pkgconf" +) + +function Invoke-Step { + param( + [string]$Name, + [scriptblock]$Command + ) + + Write-Host "==> $Name" + & $Command +} + +function Get-MissingUcrtTools { + $missing = @() + + if (-not (Test-Path (Join-Path $ucrtBin "gcc.exe")) -or -not (Test-Path (Join-Path $ucrtBin "g++.exe"))) { + $missing += "gcc/g++" + } + + if (-not (Test-Path (Join-Path $ucrtBin "clang.exe")) -or -not (Test-Path (Join-Path $ucrtBin "libclang.dll"))) { + $missing += "clang/libclang" + } + + if (-not (Test-Path (Join-Path $ucrtBin "cmake.exe"))) { + $missing += "cmake" + } + + if (-not (Test-Path (Join-Path $ucrtBin "pkgconf.exe")) -and -not (Test-Path (Join-Path $ucrtBin "pkg-config.exe"))) { + $missing += "pkgconf" + } + + $missing +} + +function Get-LatestPlatformGem { + $gem = Get-ChildItem -Path $gemPattern -ErrorAction SilentlyContinue | + Sort-Object LastWriteTime -Descending | + Select-Object -First 1 + + if (-not $gem) { + throw "No Windows GNU platform gem found at '$gemPattern'. Re-run with -BuildGem first." + } + + $gem +} + +function Invoke-RubySmoke { + param( + [string]$Name, + [string]$Code + ) + + Write-Host "Smoke: $Name" + ruby -e $Code + if ($LASTEXITCODE -ne 0) { + throw "$Name failed with exit code $LASTEXITCODE." + } +} + +Invoke-Step "Check Ruby platform" { + $rubyArch = & ruby -rrbconfig -e "print RbConfig::CONFIG['arch']" + if ($rubyArch -ne $platform) { + throw "Expected Ruby platform '$platform', got '$rubyArch'. Use RubyInstaller UCRT for this build." + } + + ruby -v + ruby -rrbconfig -rrubygems -e "puts RbConfig::CONFIG.values_at('arch', 'host', 'CC').join(%q{ }); puts Gem::Platform.local" +} + +Invoke-Step "Install Rust target" { + $installedTargets = @(rustup target list --installed) + if ($installedTargets -contains $target) { + Write-Host "Rust target $target already installed; skipping rustup." + return + } + + rustup target add $target + if ($LASTEXITCODE -ne 0) { + throw "Failed to install Rust target $target." + } +} + +Invoke-Step "Check MSYS2 UCRT build tools" { + $missing = @(Get-MissingUcrtTools) + if ($missing.Count -eq 0) { + Write-Host "MSYS2 UCRT build tools already present; skipping pacman." + return + } + + if ($SkipToolInstall) { + throw "Missing MSYS2 UCRT build tools: $($missing -join ', '). Re-run without -SkipToolInstall or install them with ridk." + } + + Write-Host "Missing MSYS2 UCRT build tools: $($missing -join ', ')" + ridk exec pacman -S --needed --noconfirm @msysPackages + if ($LASTEXITCODE -ne 0) { + $stillMissing = @(Get-MissingUcrtTools) + if ($stillMissing.Count -eq 0) { + Write-Warning "pacman returned exit code $LASTEXITCODE, but all required tools are present; continuing." + return + } + + throw "Failed to install MSYS2 UCRT build tools. Missing: $($stillMissing -join ', '). If pacman cannot lock its database, close other pacman/ridk shells, run PowerShell as Administrator, or install RubyInstaller in a user-writable directory." + } +} + +Invoke-Step "Configure Windows GNU toolchain" { + $env:CARGO_BUILD_TARGET = $target + $env:LIBCLANG_PATH = $ucrtBin + $env:CC_x86_64_pc_windows_gnu = Join-Path $ucrtBin "gcc.exe" + $env:CXX_x86_64_pc_windows_gnu = Join-Path $ucrtBin "g++.exe" + $env:CMAKE = Join-Path $ucrtBin "cmake.exe" + Remove-Item Env:\CMAKE_GENERATOR -ErrorAction SilentlyContinue + + rustc -Vv + Write-Host "LIBCLANG_PATH=$env:LIBCLANG_PATH" + Write-Host "CC_x86_64_pc_windows_gnu=$env:CC_x86_64_pc_windows_gnu" + Write-Host "CXX_x86_64_pc_windows_gnu=$env:CXX_x86_64_pc_windows_gnu" + Write-Host "CMAKE=$env:CMAKE" +} + +if (-not $SkipBundleInstall) { + Invoke-Step "Install Ruby dependencies" { + bundle install + } +} + +Invoke-Step "Compile native extension" { + ridk exec ruby -S bundle exec rake compile +} + +Invoke-Step "Verify extension loads" { + ruby -Ilib -rwreq -e "puts Wreq::VERSION; puts Wreq::Client.new.class" +} + +if ($RunTests) { + Invoke-Step "Run tests" { + ridk exec ruby -S bundle exec rake test + } +} + +if ($BuildGem) { + Invoke-Step "Build local platform gem" { + $rubyMinor = & ruby -e "print RUBY_VERSION[/\A\d+\.\d+/]" + $dest = "lib\wreq_ruby\$rubyMinor" + New-Item -ItemType Directory -Force $dest | Out-Null + Copy-Item "lib\wreq_ruby\wreq_ruby.so" (Join-Path $dest "wreq_ruby.so") -Force + + ruby script/build_platform_gem.rb $platform + } +} + +if ($TestGem) { + Invoke-Step "Install and smoke test platform gem" { + $gem = Get-LatestPlatformGem + $gemHome = Join-Path ([System.IO.Path]::GetTempPath()) "wreq-gem-test-$PID" + $gemBin = Join-Path $gemHome "bin" + $testDir = Join-Path ([System.IO.Path]::GetTempPath()) "wreq-gem-test-cwd-$PID" + + Remove-Item -Recurse -Force $gemHome, $testDir -ErrorAction SilentlyContinue + New-Item -ItemType Directory -Force $gemHome, $gemBin, $testDir | Out-Null + + $oldGemHome = $env:GEM_HOME + $oldGemPath = $env:GEM_PATH + $oldGemrc = $env:GEMRC + $oldRubyopt = $env:RUBYOPT + $oldRubylib = $env:RUBYLIB + $oldBundleGemfile = $env:BUNDLE_GEMFILE + $oldBundleBinPath = $env:BUNDLE_BIN_PATH + $oldBundlerVersion = $env:BUNDLER_VERSION + $oldSmokeUrl = $env:WREQ_SMOKE_URL + $oldWreqGemHome = $env:WREQ_GEM_HOME + + try { + $env:GEM_HOME = $gemHome + $env:GEM_PATH = $gemHome + $env:GEMRC = "" + $env:RUBYOPT = "" + $env:RUBYLIB = "" + $env:BUNDLE_GEMFILE = "" + $env:BUNDLE_BIN_PATH = "" + $env:BUNDLER_VERSION = "" + $env:WREQ_SMOKE_URL = $SmokeUrl + $env:WREQ_GEM_HOME = $gemHome + + gem install --norc --local --install-dir $gemHome --bindir $gemBin --no-document --force --no-user-install $gem.FullName + if ($LASTEXITCODE -ne 0) { + throw "Failed to install $($gem.FullName)." + } + + Push-Location $testDir + try { + Invoke-RubySmoke "Load installed platform gem" @' +STDOUT.sync = true +STDERR.sync = true +require "wreq" +spec = Gem.loaded_specs.fetch("wreq") +expected = File.expand_path(ENV.fetch("WREQ_GEM_HOME")) +actual = File.expand_path(spec.full_gem_path) +raise "loaded #{actual}, expected under #{expected}" unless actual.start_with?(expected) +puts "loaded #{spec.full_name}" +puts actual +puts "wreq #{Wreq::VERSION}" +'@ + + Invoke-RubySmoke "Request through installed platform gem" @' +STDOUT.sync = true +STDERR.sync = true +require "wreq" +client = Wreq::Client.new +resp = client.get(ENV.fetch("WREQ_SMOKE_URL"), timeout: 20) +puts "HTTP #{resp.code}" +raise "unexpected status #{resp.code}" unless resp.code == 200 +'@ + } finally { + Pop-Location + } + } finally { + $env:GEM_HOME = $oldGemHome + $env:GEM_PATH = $oldGemPath + $env:GEMRC = $oldGemrc + $env:RUBYOPT = $oldRubyopt + $env:RUBYLIB = $oldRubylib + $env:BUNDLE_GEMFILE = $oldBundleGemfile + $env:BUNDLE_BIN_PATH = $oldBundleBinPath + $env:BUNDLER_VERSION = $oldBundlerVersion + $env:WREQ_SMOKE_URL = $oldSmokeUrl + $env:WREQ_GEM_HOME = $oldWreqGemHome + Remove-Item -Recurse -Force $gemHome, $testDir -ErrorAction SilentlyContinue + } + } +} diff --git a/src/arch.rs b/src/arch.rs new file mode 100644 index 0000000..7c17a01 --- /dev/null +++ b/src/arch.rs @@ -0,0 +1,33 @@ +//! Platform-specific support code. +//! +//! Keep this module small and focused on platform quirks that affect linking, +//! ABI boundaries, or OS APIs used by the Rust extension. Normal HTTP client +//! behavior should stay in the client/runtime modules so platform workarounds +//! do not leak into the rest of the binding. + +#[cfg(all(target_os = "windows", target_env = "gnu"))] +mod windows_gnu { + //! Windows GNU support. + //! + //! RubyInstaller's GNU/UCRT Ruby exports some symbols with the same names as + //! Win32 APIs. When GNU ld links this extension against Ruby, those symbols + //! can shadow the real Windows APIs unless we pin the calls we care about. + + #[link(name = "kernel32")] + unsafe extern "system" { + fn SleepEx(milliseconds: u32, alertable: i32) -> u32; + } + + // RubyInstaller's GNU/UCRT Ruby exports a Ruby-aware Sleep symbol. GNU ld can + // bind extension calls to that symbol instead of KERNEL32!Sleep, which crashes + // on native worker threads that have no Ruby TLS. The linker wrapper keeps those + // calls on the Windows API path. + #[unsafe(no_mangle)] + pub extern "system" fn __wrap_Sleep(milliseconds: u32) { + // SAFETY: SleepEx is a Win32 API. Passing alertable=false matches Sleep's + // behavior, and any u32 duration is accepted by the API. + unsafe { + SleepEx(milliseconds, 0); + } + } +} diff --git a/src/client.rs b/src/client.rs index 7ef7b95..ac0bc4e 100644 --- a/src/client.rs +++ b/src/client.rs @@ -104,6 +104,7 @@ struct Builder { /// Bind to a local IP Address. local_address: Option, /// Bind to an interface by `SO_BINDTODEVICE`. + #[allow(dead_code)] interface: Option, // ========= Compression options ========= @@ -300,6 +301,18 @@ impl Client { apply_option!(set_if_some, builder, params.proxy, proxy); apply_option!(set_if_true, builder, params.no_proxy, no_proxy, false); apply_option!(set_if_some, builder, params.local_address, local_address); + #[cfg(any( + target_os = "android", + target_os = "fuchsia", + target_os = "illumos", + target_os = "ios", + target_os = "linux", + target_os = "macos", + target_os = "solaris", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ))] apply_option!(set_if_some, builder, params.interface, interface); // Compression options. diff --git a/src/client/body/stream.rs b/src/client/body/stream.rs index d34e8c4..1146c36 100644 --- a/src/client/body/stream.rs +++ b/src/client/body/stream.rs @@ -5,7 +5,7 @@ use std::{ }; use bytes::Bytes; -use futures_util::{Stream, StreamExt, TryFutureExt}; +use futures_util::{Stream, StreamExt}; use magnus::{Error, RString, TryConvert, Value}; use tokio::sync::{ Mutex, @@ -40,13 +40,16 @@ impl BodyReceiver { /// Read the next body chunk, converting stream errors into Ruby errors. pub fn next(&self) -> Result, Error> { - rt::try_block_on(async { - match self.0.lock().await.as_mut().next().await { - Some(Ok(data)) => Ok(Some(data)), - Some(Err(err)) => Err(wreq_error_to_magnus(err)), - None => Ok(None), - } - }) + rt::try_block_on( + async { + match self.0.lock().await.as_mut().next().await { + Some(Ok(data)) => Ok(Some(data)), + Some(Err(err)) => Err(err), + None => Ok(None), + } + }, + wreq_error_to_magnus, + ) } } @@ -73,7 +76,7 @@ impl BodySender { let bytes = data.to_bytes(); let inner = rb_self.0.read().unwrap(); if let Some(ref tx) = inner.tx { - rt::try_block_on(tx.send(bytes).map_err(mpsc_send_error_to_magnus))?; + rt::try_block_on(tx.send(bytes), mpsc_send_error_to_magnus)?; } Ok(()) } diff --git a/src/client/req.rs b/src/client/req.rs index 45b4547..ba7f6d5 100644 --- a/src/client/req.rs +++ b/src/client/req.rs @@ -33,6 +33,7 @@ pub struct Request { local_address: Option, /// Bind to an interface by `SO_BINDTODEVICE`. + #[allow(dead_code)] interface: Option, /// The timeout to use for the request. @@ -144,119 +145,130 @@ pub fn execute_request>( url: U, mut request: Request, ) -> Result { - rt::try_block_on(async move { - let mut builder = client.request(method.into_ffi(), url.as_ref()); - - // Emulation options. - apply_option!(set_if_some_inner, builder, request.emulation, emulation); - - // Version options. - apply_option!( - set_if_some_map, - builder, - request.version, - version, - Version::into_ffi - ); - - // Timeout options. - apply_option!( - set_if_some_map, - builder, - request.timeout, - timeout, - Duration::from_secs - ); - apply_option!( - set_if_some_map, - builder, - request.read_timeout, - read_timeout, - Duration::from_secs - ); - - // Network options. - apply_option!(set_if_some, builder, request.proxy, proxy); - apply_option!(set_if_some, builder, request.local_address, local_address); - apply_option!(set_if_some, builder, request.interface, interface); - - // Headers options. - apply_option!(set_if_some_into_inner, builder, request.headers, headers); - apply_option!( - set_if_some_inner, - builder, - request.orig_headers, - orig_headers - ); - apply_option!( - set_if_some, - builder, - request.default_headers, - default_headers - ); - - // Cookies options. - if let Some(cookies) = request.cookies.take() { - for cookie in cookies.0 { - builder = builder.header(header::COOKIE, cookie); + rt::try_block_on( + async move { + let mut builder = client.request(method.into_ffi(), url.as_ref()); + + // Emulation options. + apply_option!(set_if_some_inner, builder, request.emulation, emulation); + + // Version options. + apply_option!( + set_if_some_map, + builder, + request.version, + version, + Version::into_ffi + ); + + // Timeout options. + apply_option!( + set_if_some_map, + builder, + request.timeout, + timeout, + Duration::from_secs + ); + apply_option!( + set_if_some_map, + builder, + request.read_timeout, + read_timeout, + Duration::from_secs + ); + + // Network options. + apply_option!(set_if_some, builder, request.proxy, proxy); + apply_option!(set_if_some, builder, request.local_address, local_address); + #[cfg(any( + target_os = "android", + target_os = "fuchsia", + target_os = "illumos", + target_os = "ios", + target_os = "linux", + target_os = "macos", + target_os = "solaris", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ))] + apply_option!(set_if_some, builder, request.interface, interface); + + // Headers options. + apply_option!(set_if_some_into_inner, builder, request.headers, headers); + apply_option!( + set_if_some_inner, + builder, + request.orig_headers, + orig_headers + ); + apply_option!( + set_if_some, + builder, + request.default_headers, + default_headers + ); + + // Cookies options. + if let Some(cookies) = request.cookies.take() { + for cookie in cookies.0 { + builder = builder.header(header::COOKIE, cookie); + } } - } - - // Authentication options. - apply_option!( - set_if_some_map_ref, - builder, - request.auth, - auth, - AsRef::::as_ref - ); - apply_option!(set_if_some, builder, request.bearer_auth, bearer_auth); - if let Some(basic_auth) = request.basic_auth.take() { - builder = builder.basic_auth(basic_auth.0, basic_auth.1); - } - // Allow redirects options. - match request.allow_redirects { - Some(false) => { - builder = builder.redirect(wreq::redirect::Policy::none()); + // Authentication options. + apply_option!( + set_if_some_map_ref, + builder, + request.auth, + auth, + AsRef::::as_ref + ); + apply_option!(set_if_some, builder, request.bearer_auth, bearer_auth); + if let Some(basic_auth) = request.basic_auth.take() { + builder = builder.basic_auth(basic_auth.0, basic_auth.1); } - Some(true) => { - builder = builder.redirect( - request - .max_redirects - .take() - .map(wreq::redirect::Policy::limited) - .unwrap_or_default(), - ); - } - None => {} - }; - - // Compression options. - apply_option!(set_if_some, builder, request.gzip, gzip); - apply_option!(set_if_some, builder, request.brotli, brotli); - apply_option!(set_if_some, builder, request.deflate, deflate); - apply_option!(set_if_some, builder, request.zstd, zstd); - // Query options. - apply_option!(set_if_some_ref, builder, request.query, query); - - // Form options. - apply_option!(set_if_some_ref, builder, request.form, form); - - // JSON options. - apply_option!(set_if_some_ref, builder, request.json, json); - - // Body options. - if let Some(body) = request.body.take() { - builder = builder.body(wreq::Body::from(body)); - } + // Allow redirects options. + match request.allow_redirects { + Some(false) => { + builder = builder.redirect(wreq::redirect::Policy::none()); + } + Some(true) => { + builder = builder.redirect( + request + .max_redirects + .take() + .map(wreq::redirect::Policy::limited) + .unwrap_or_default(), + ); + } + None => {} + }; + + // Compression options. + apply_option!(set_if_some, builder, request.gzip, gzip); + apply_option!(set_if_some, builder, request.brotli, brotli); + apply_option!(set_if_some, builder, request.deflate, deflate); + apply_option!(set_if_some, builder, request.zstd, zstd); + + // Query options. + apply_option!(set_if_some_ref, builder, request.query, query); + + // Form options. + apply_option!(set_if_some_ref, builder, request.form, form); + + // JSON options. + apply_option!(set_if_some_ref, builder, request.json, json); + + // Body options. + if let Some(body) = request.body.take() { + builder = builder.body(wreq::Body::from(body)); + } - // Send request. - builder - .send() - .await - .map(Response::new) - .map_err(wreq_error_to_magnus) - }) + // Send request. + builder.send().await.map(Response::new) + }, + wreq_error_to_magnus, + ) } diff --git a/src/client/resp.rs b/src/client/resp.rs index ce35057..ec28291 100644 --- a/src/client/resp.rs +++ b/src/client/resp.rs @@ -81,9 +81,8 @@ impl Response { Ok(build_response(body)) } else { let bytes = rt::try_block_on( - BodyExt::collect(body) - .map_ok(|buf| buf.to_bytes()) - .map_err(wreq_error_to_magnus), + BodyExt::collect(body).map_ok(|buf| buf.to_bytes()), + wreq_error_to_magnus, )?; self.body @@ -170,7 +169,7 @@ impl Response { /// Get the response body as bytes. pub fn bytes(&self) -> Result { let response = self.response(false)?; - rt::try_block_on(response.bytes().map_err(wreq_error_to_magnus)) + rt::try_block_on(response.bytes(), wreq_error_to_magnus) } /// Get the full response text given a specific encoding. @@ -178,25 +177,18 @@ impl Response { let args = scan_args::<(), (Option,), (), (), (), ()>(args)?; let response = self.response(false)?; match args.optional.0 { - Some(encoding) => rt::try_block_on( - response - .text_with_charset(encoding) - .map_err(wreq_error_to_magnus), - ), - None => rt::try_block_on(response.text().map_err(wreq_error_to_magnus)), + Some(encoding) => { + rt::try_block_on(response.text_with_charset(encoding), wreq_error_to_magnus) + } + None => rt::try_block_on(response.text(), wreq_error_to_magnus), } } /// Get the response body as JSON. pub fn json(ruby: &Ruby, rb_self: &Self) -> Result { let response = rb_self.response(false)?; - rt::try_block_on(async move { - let json = response - .json::() - .await - .map_err(wreq_error_to_magnus)?; - serde_magnus::serialize(ruby, &json) - }) + let json = rt::try_block_on(response.json::(), wreq_error_to_magnus)?; + serde_magnus::serialize(ruby, &json) } /// Yield response body chunks to the given Ruby block. diff --git a/src/lib.rs b/src/lib.rs index 3ef649d..5d0db8f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ #[macro_use] mod macros; +mod arch; mod client; mod cookie; mod emulate; diff --git a/src/rt.rs b/src/rt.rs index ebedd6f..9cc96cb 100644 --- a/src/rt.rs +++ b/src/rt.rs @@ -5,25 +5,42 @@ use tokio::runtime::{Builder, Runtime}; use crate::{error::interrupt_error, gvl}; static RUNTIME: LazyLock = LazyLock::new(|| { - Builder::new_multi_thread() + let mut builder = Builder::new_multi_thread(); + + builder .enable_all() .build() .expect("Failed to initialize Tokio runtime") }); -/// Block on a future to completion on the global Tokio runtime, -/// with support for cancellation via the provided `CancelFlag`. -pub fn try_block_on(future: F) -> F::Output +enum BlockOnError { + Interrupted, + Future(E), +} + +/// Block on a future to completion on the 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 +/// been reacquired. +pub fn try_block_on(future: F, map_err: M) -> Result where - F: Future>, + F: Future>, + M: FnOnce(E) -> magnus::Error, { - gvl::nogvl_cancellable(|flag| { + let result = gvl::nogvl_cancellable(|flag| { RUNTIME.block_on(async move { tokio::select! { biased; - _ = flag.cancelled() => Err(interrupt_error()), - result = future => result, + _ = flag.cancelled() => Err(BlockOnError::Interrupted), + result = future => result.map_err(BlockOnError::Future), } }) - }) + }); + + match result { + Ok(value) => Ok(value), + Err(BlockOnError::Interrupted) => Err(interrupt_error()), + Err(BlockOnError::Future(err)) => Err(map_err(err)), + } }