Seahorse http connection pool may crash after fork(2)

[yokonao]

Describe the bug

If you call fork(2) when multi-threaded AWS API calls, some error may occur in the child process.
RuntimeError: can't add a new key into hash during iteration occurs as reported in #1331.

I think calling Aws.empty_connection_pools! after fork will prevent this crash, but resetting the connection pool using Aws.empty_connection_pools! may be incomplete.
In Aws.empty_connection_pools! we call Hash#clear on the hash that is pooling the http connection, but Hash#clear does not reset the state that it is iterating.
ref. https://github.com/ruby/ruby/blob/v3_1_5/hash.c#L2867-L2869
So, it may crash even if Aws.empty_connection_pools! is executed immediately after fork(2).

Expected Behavior

We can prevent RuntimeError: can't add a new key into hash during iteration by discarding the seahorse connection pool after fork(2).

Current Behavior

RuntimeError: can't add a new key into hash during iteration may occurs even if Aws.empty_connection_pools! is executed immediately after fork(2).

Reproduction Steps

A simple ruby script for reproduction.

require 'aws-sdk-s3'
require 'aws-sdk-dynamodb'

Aws::S3::Client.new.list_buckets

# iterate Seahorse::Client::NetHttp::ConnectionPool @pool forever
Thread.new { loop { Seahorse::Client::NetHttp::ConnectionPool.pools.first.size } }

sleep 0.1 # wait a miniute for the thread to start

fork do
  Aws.empty_connection_pools!

  Aws::DynamoDB::Client.new.list_tables
end

• In the parent process, the connection pool hash is iterated in another thread
  • The size method has been changed to not cause iteration in Change ConnectionPool size to not use an iterator #3015, but there are some methods that cause iteration
• Aws.empty_connection_pools! is called after fork, and call AWS API (push a new connection into the pool internally)

Executing the above script may causes RuntimeError: can't add a new key into hash during iteration.

I checked with the following versions.

• aws-sdk-core 3.170.0
• aws-sdk-s3 1.106.0
• aws-sdk-dynamodb 1.84.0

Possible Solution

More radical destruction is required to ensure that the connection pooling Hash is not reused in child processes.

I have confirmed that methods like below are effective to prevent the error

class Seahorse::Client::NetHttp::ConnectionPool
  def self.discard_connection_pools
    @pools_mutex.synchronize { @pools = {} }
  end
end

Adding this method to the reproduction script:

require 'aws-sdk-s3'
require 'aws-sdk-dynamodb'

class Seahorse::Client::NetHttp::ConnectionPool
  def self.discard_connection_pools
    @pools_mutex.synchronize { @pools = {} }
  end
end

Aws::S3::Client.new.list_buckets

# iterate Seahorse::Client::NetHttp::ConnectionPool @pool forever
Thread.new { loop { Seahorse::Client::NetHttp::ConnectionPool.pools.first.size } }

sleep 0.1 # wait a miniute for the thread to start

fork do
  Seahorse::Client::NetHttp::ConnectionPool.discard_connection_pools

  Aws::DynamoDB::Client.new.list_tables
end

I have confirmed that no errors occur even after several hundred runs.

Additional Information/Context

No response

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-core

Environment details (Version of Ruby, OS environment)

Ruby 3.1 (Our environment)

[mullermp]

Thanks for opening an issue. I think we could fix the empty! method to not use iteration as well. We will investigate.

[jterapin]

Hi! We have a pending PR in efforts to resolve this issue. I will let you know when it is merged.

I did noticed that you are using an older version of the aws-sdk-core (3.170.0). That version of the core will not have changes that was done for the previously mentioned fix.

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

[jterapin]

This fix will be released with tomorrow's update

[yokonao]

Thank you!
Does this mean that there is no possibility of the connection pool hash being iterated?

[jterapin]

Yes, that was our intention. We did an audit of the ConnectionPool but let us know if you come across any issues after using tomorrow's release.

[mullermp]

Does this mean that there is no possibility of the connection pool hash being iterated?

I think it will always be iterated, but I think the difference of whether it happens in C or Ruby when using fork is what matters. I think this should fix it for you!

Thanks @jterapin for picking this up while I am away at RubyKaigi :)