Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compatibility with connection_pool 2.4+ #144

Merged
merged 2 commits into from
Mar 29, 2023

Conversation

casperisfine
Copy link

Fix: mperham/connection_pool#172
Ref: mperham/connection_pool#166

ConnectionPool descendants must accept checking(force: true).

When it's called, all the cached connection should be checked back in, and the Thread.current state reset.

Additionally, making Connection respond to close allow them to be automatically closed on fork.

@drbrain @tenderlove

Fix: mperham/connection_pool#172
Ref: mperham/connection_pool#166

ConnectionPool descendants must accept `checking(force: true)`.

When it's called, all the cached connection should be checked back in,
and the `Thread.current` state reset.

Additionally, making `Connection` respond to `close` allow them
to be automatically closed on fork.
@tisba
Copy link
Contributor

tisba commented Mar 29, 2023

The breaking tests seem to be quite unrelated to your change, @casperisfine 🤔

@tenderworks
Copy link

Looks like an error message was changed. @casperisfine would you mind updating the test to an assert_match or something?

@tisba
Copy link
Contributor

tisba commented Mar 29, 2023

@tenderworks @tenderlove I found it #145, can you trigger the build?

* master:
  Use `assert_match`
  Update test_net_http_persistent_timed_stack_multi.rb
@tenderlove tenderlove merged commit fc3a7db into drbrain:master Mar 29, 2023
@tisba
Copy link
Contributor

tisba commented Mar 30, 2023

I was reproducing related issue we had in prod because of the issues that's been fixed here, and I thought I share how this case can be reproduced. Happy to confirm that 4.0.2 does indeed fix the issue.

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  
  gem "parallel", "1.22.1"
  gem "connection_pool", "2.4.0"

  # bad
  gem "net-http-persistent", "4.0.1"

  # good
  # gem "net-http-persistent", "4.0.2"
end

require "parallel"
require "net/http/persistent"

Net::HTTP::Persistent.new(name: "my_app_name").request(URI('http://example.com/awesome/web/service'))

puts "Parent: #{Process.pid}"
Parallel.map([42], in_processes: 1) do
  puts "Child: #{Process.pid}"
  sleep 0.1
end

This will crash with 4.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

net_http_args nil class exception
5 participants