curl easy handles leaked when response callback returns error #679

Roguelazer · 2021-07-23T01:15:23Z

We recently upgraded one of our applications from Faraday 0.15.3 and Typhoeus 1.3 to Faraday 1.5.1 and Typhoeus 1.4; it appears that Typhoeus now leaks a curl_easy object on many invocation. Since cURL 7.68, in addition to leaking some memory, each curl_multi object leaks a pair of file descriptors (from the wakeup socketpair), which makes this bug, uh, rather more impactful for us.

We did some investigation, and the flow is basically the following:

Faraday::Adapter::Typhoeus creates a Typhoeus::Request object and calls .run on it. It also sets up some on_complete callbacks to turn Typhoeus exceptions into Faraday exceptions (which means that the on_complete Typhoeus callback always raises an exception when invoked from Faraday if the underlying request failed).
Typhoeus::Request::Operations.run calls EasyFactory.new(self).get to get an Easy
Typhoeus::EasyFactory.set_callback releases the easy back into the pool if and only if request.finish(Response.new(...)) does not raise any exceptions. Otherwise, it just leaks the easy object.

This is fairly reproducible without involving Faraday using the following sample program:

require 'typhoeus'

def list_open_fds
  Dir.children("/proc/self/fd")
end

puts list_open_fds
(0..10).each do
  begin
    req = Typhoeus::Request.new("http://nonexistent.internal")
    req.on_complete do
      raise RuntimeError.new("poop")
    end
    resp = req.run
    puts resp
  rescue
  end
end
puts list_open_fds

On a system with cURL 7.68.0 or later, this will show a leak of 20 file descriptors.

Doing a major garbage collect (with GC.start, or by leaking enough RAM elsewhere in the program) will free the easy object, but for some reason they are never freed in a minor garbage collection. I'm not sure why this is; maybe it's some artifact of how FFI::AutoPointer works.

A couple of things that seem like possible fixes:

Option 1: always release the easy back into the pool, even if there are errors. Is this safe? I'm not sure. It doesn't seem like any of the errors from the response callback should result in the easy handle itself being in a bad state...

diff --git a/lib/typhoeus/easy_factory.rb b/lib/typhoeus/easy_factory.rb
index 84c7131..3c820f0 100644
--- a/lib/typhoeus/easy_factory.rb
+++ b/lib/typhoeus/easy_factory.rb
@@ -161,8 +161,11 @@ module Typhoeus
         end
       end
       easy.on_complete do |easy|
-        request.finish(Response.new(easy.mirror.options))
-        Typhoeus::Pool.release(easy)
+        begin
+          request.finish(Response.new(easy.mirror.options))
+        ensure
+          Typhoeus::Pool.release(easy)
+        end
         if hydra && !hydra.queued_requests.empty?
           hydra.dequeue_many
         end

Option 2: always immediately free the easy if there are errors:

diff --git a/lib/typhoeus/easy_factory.rb b/lib/typhoeus/easy_factory.rb
index 84c7131..fc08618 100644
--- a/lib/typhoeus/easy_factory.rb
+++ b/lib/typhoeus/easy_factory.rb
@@ -161,7 +161,12 @@ module Typhoeus
         end
       end
       easy.on_complete do |easy|
-        request.finish(Response.new(easy.mirror.options))
+        begin
+          request.finish(Response.new(easy.mirror.options))
+        rescue
+          easy.cleanup
+          raise
+        end
         Typhoeus::Pool.release(easy)
         if hydra && !hydra.queued_requests.empty?
           hydra.dequeue_many

Both of these appear to fix the problem in our application.

For now I'm working around this by doing a full GC.start every time Faraday raises an exception, but that's not a great solution.

The text was updated successfully, but these errors were encountered:

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

curl easy handles leaked when response callback returns error #679

curl easy handles leaked when response callback returns error #679

curl easy handles leaked when response callback returns error #679

curl easy handles leaked when response callback returns error #679

Comments