From 1dbb048a3a7e1fe1bd9e488f97cf13681a94acd0 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Mon, 24 Nov 2025 21:18:34 +0200 Subject: [PATCH 1/2] Fix HTTPX thread-safety issue with thread-local connections for incremental rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: The HTTPX stream_bidi plugin has a critical limitation - only the thread that creates the bidirectional streaming connection can write chunks to the request stream. This causes race conditions in multi-threaded production environments (e.g., Puma) where different HTTP requests run on different threads. When the first request creates a connection on Thread A and stores it in an instance variable, subsequent requests on Thread B cannot write to that connection, causing connection errors and unpredictable behavior. Solution: Implement thread-local storage for incremental rendering connections. Each thread now gets its own persistent bidirectional streaming connection stored in Thread.current instead of a shared instance variable. Changes: - Modified `incremental_connection` to use `Thread.current[:react_on_rails_incremental_connection]` - Added `reset_thread_local_incremental_connections` to properly clean up all thread-local connections - Updated `reset_connection` to call the new cleanup method - Removed `@incremental_connection` instance variable Trade-offs: - ✅ Eliminates race conditions and thread-safety issues - ✅ Simple implementation using Ruby's built-in thread-local storage - ✅ Each thread has isolated, persistent connection - ⚠️ Higher memory usage (one connection per Puma worker thread) - ⚠️ Not optimal for high-scale production with many threads This is a temporary solution. Long-term options: 1. Fix HTTPX to support multi-threaded bidirectional streaming 2. Migrate to async-http gem which is thread-safe by design Fixes #2115 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../lib/react_on_rails_pro/request.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/react_on_rails_pro/lib/react_on_rails_pro/request.rb b/react_on_rails_pro/lib/react_on_rails_pro/request.rb index cc704bf48a..b11be71f3e 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/request.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/request.rb @@ -10,9 +10,8 @@ class Request # rubocop:disable Metrics/ClassLength class << self def reset_connection @standard_connection&.close - @incremental_connection&.close @standard_connection = nil - @incremental_connection = nil + reset_thread_local_incremental_connections end def render_code(path, js_code, send_bundle) @@ -135,8 +134,19 @@ def connection end # rubocop:enable Naming/MemoizedInstanceVariableName + # Thread-local connection for incremental rendering + # Each thread gets its own persistent connection to avoid connection pool issues def incremental_connection - @incremental_connection ||= create_incremental_connection + Thread.current[:react_on_rails_incremental_connection] ||= create_incremental_connection + end + + def reset_thread_local_incremental_connections + # Close all thread-local incremental connections + Thread.list.each do |thread| + conn = thread[:react_on_rails_incremental_connection] + conn&.close + thread[:react_on_rails_incremental_connection] = nil + end end def perform_request(path, **post_options) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity From 87fb7f8cb6f272eef48a24c38260774c91b036e5 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Mon, 24 Nov 2025 21:20:13 +0200 Subject: [PATCH 2/2] Refactor incremental rendering to use async handling for update chunks --- packages/react-on-rails-pro-node-renderer/src/worker.ts | 4 ++-- .../src/worker/handleIncrementalRenderRequest.ts | 6 +++--- packages/react-on-rails-pro-node-renderer/src/worker/vm.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-on-rails-pro-node-renderer/src/worker.ts b/packages/react-on-rails-pro-node-renderer/src/worker.ts index eeda4a1f8f..a7b862f0f4 100644 --- a/packages/react-on-rails-pro-node-renderer/src/worker.ts +++ b/packages/react-on-rails-pro-node-renderer/src/worker.ts @@ -315,7 +315,7 @@ export default function run(config: Partial) { } }, - onUpdateReceived: (obj: unknown) => { + onUpdateReceived: async (obj: unknown) => { if (!incrementalSink) { log.error({ msg: 'Unexpected update chunk received after rendering was aborted', obj }); return; @@ -323,7 +323,7 @@ export default function run(config: Partial) { try { log.info(`Received a new update chunk ${JSON.stringify(obj)}`); - incrementalSink.add(obj); + await incrementalSink.add(obj); } catch (err) { // Log error but don't stop processing log.error({ err, msg: 'Error processing update chunk' }); diff --git a/packages/react-on-rails-pro-node-renderer/src/worker/handleIncrementalRenderRequest.ts b/packages/react-on-rails-pro-node-renderer/src/worker/handleIncrementalRenderRequest.ts index 40b0b5515a..0ebc8b7189 100644 --- a/packages/react-on-rails-pro-node-renderer/src/worker/handleIncrementalRenderRequest.ts +++ b/packages/react-on-rails-pro-node-renderer/src/worker/handleIncrementalRenderRequest.ts @@ -5,7 +5,7 @@ import { getRequestBundleFilePath } from '../shared/utils'; export type IncrementalRenderSink = { /** Called for every subsequent NDJSON object after the first one */ - add: (chunk: unknown) => void; + add: (chunk: unknown) => Promise; handleRequestClosed: () => void; }; @@ -93,11 +93,11 @@ export async function handleIncrementalRenderRequest( return { response, sink: { - add: (chunk: unknown) => { + add: async (chunk: unknown) => { try { assertIsUpdateChunk(chunk); const bundlePath = getRequestBundleFilePath(chunk.bundleTimestamp); - executionContext.runInVM(chunk.updateChunk, bundlePath).catch((err: unknown) => { + await executionContext.runInVM(chunk.updateChunk, bundlePath).catch((err: unknown) => { log.error({ msg: 'Error running incremental render chunk', err, chunk }); }); } catch (err) { diff --git a/packages/react-on-rails-pro-node-renderer/src/worker/vm.ts b/packages/react-on-rails-pro-node-renderer/src/worker/vm.ts index f2e27eeee2..8dfac551bd 100644 --- a/packages/react-on-rails-pro-node-renderer/src/worker/vm.ts +++ b/packages/react-on-rails-pro-node-renderer/src/worker/vm.ts @@ -365,7 +365,7 @@ export async function buildExecutionContext( const objectResult = await result; result = JSON.stringify(objectResult); } - if (log.level === 'debug') { + if (log.level === 'debug' && result) { log.debug(`result from JS: ${smartTrim(result)}`); const debugOutputPathResult = path.join(serverBundleCachePath, 'result.json');