`options` hash is not thread-safe in multi-threaded environments (e.g., Puma)

[muk-ai]

Description

Problem

In multi-threaded environments like Puma, the options hash is not thread-safe and can cause race conditions when multiple concurrent requests modify it. This leads to incorrect responses being returned to clients.

Root Cause

When Grape creates endpoint instances for each request, it uses dup to copy the instance. However, this performs a shallow copy of the options hash. As a result:

1. Multiple concurrent requests to the same endpoint share the same options hash object
2. When one request modifies options (e.g., options[:include] = params[:include]), it affects other concurrent requests
3. This creates race conditions where a request may receive a response with incorrect options set by another request

Steps to Reproduce

1. Set up a Grape API with Puma in multi-threaded mode (e.g., threads 5, 5)
2. Create an endpoint that modifies options:

get '/works' do
  options[:include] = params[:include]  # Modifies shared options hash
  # ... process request
  render @works
end

3. Send multiple concurrent requests to the same endpoint with different include parameters:
   • Request A: GET /works?include=author
   • Request B: GET /works?include=comments
4. Observe that some responses have incorrect include values due to race conditions

Expected Behavior

Each request should have its own isolated options hash that doesn't affect other concurrent requests.

Current Workaround

We currently work around this by deep-copying options in a before hook:

before do
  @options = options.deep_dup
end

However, this is something that should be handled by Grape itself to ensure thread-safety by default.

Environment

• Grape version: 2.4.0
• Ruby version: 3.4.2
• Web server: Puma (multi-threaded)
• OS: Linux

[ericproulx]

@muk-ai thank you for opening this issue :) Could you test it on the master branch (we should rename it to main) as I've recently merged a PR that uses kwargs instead of extract_options. Thanks.

[muk-ai]

@ericproulx
Thanks for pointing out that PR!

I believe the root cause is the shallow copy issue, so I don't think kwargs alone would solve it: https://github.com/ruby-grape/grape/blob/07fcf03e8c096e8f8016c8fc372c7f904e478e79/lib/grape/endpoint.rb#L172-L175

However, I'll test with master (3.0.0) to verify whether the issue still reproduces, just to be certain.

[ericproulx]

@muk-ai, I've been playing around with this and I found a way to reproduce it, nice finding 🎉. Also found a way to fix it but it needs more testing. Could you share your use case as why you need to modify options in the endpoint ? I just wondering why a local variable does not fit your need.

[muk-ai]

@ericproulx Thank you for taking the time to look into this issue!

Use cases for modifying options in our codebase:

1. options[:include]

We use this to pass the client-requested include parameter to ActiveModelSerializers: (Please ignore the security concerns of this code - this is simplified for illustration purposes)

get '/works' do
  options[:include] = params[:include]
  # ...
  render @works
end

I think this pattern can be rewritten as shown below. It looks good.

get '/works' do
  include = params[:include]
  # ...
  render @works, include:
end

However, I believe it's problematic that the code in before appears to work without issues. Most people probably aren't aware of thread-safety concerns.

2. options[:serialization_context]

We set serialization context for ActiveModelSerializers only when the request expects JSON:API (application/vnd.api+json).

def before
  if env['api.format'] == :json_api
    options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(
      request_url: request.url,
      query_parameters: {}
    )
  end
end

The Content-Type expected by the frontend may vary for each request.

[ericproulx]

I found a way to have a thread-safe options but it will be a shallow copy since users should not update Grape's internal keys. On the other hand, I thinking about adding a new thread-safe hash which could be used like you do.

[ericproulx]

Instead of using options, you could use env which is rack's hash. The latter is thread-safe.

[muk-ai]

Looking at the two examples I provided, both use cases were for communicating with ActiveModelSerializers through options. If we can achieve the same thing using env, that would work for us. However, that would likely require changes on the active_model_serializers side as well.

[ericproulx]

I think AMS is already using env

[muk-ai]

@ericproulx Oh! So using env instead of options was the correct way to communicate with ActiveModelSerializers all along. Thank you for letting me know!