Connection#pause doesn't work on EM::popen'd connections #245

Closed
rtomayko opened this Issue Sep 1, 2011 · 2 comments

2 participants

@rtomayko

Calling #pause on a connection created with EM::popen doesn't stop receive_data events from firing and the paused? method always returns false. Servers that fork/exec a child process and pump data from the child's stdout to a slow client have no choice but to buffer the entire output into memory.

Here's an example program (em-popen-pause-bug.rb) that reproduces the issue by popen'ing a cat /dev/random process and pausing after 500K of data has been read:

require 'eventmachine'

module PopenConnection
  def post_init
    @rec = 0
  end

  def receive_data(data)
    puts "receive_data: #{data.size}"
    @rec += data.size
    if !paused? && @rec > 500 * 1024
      puts "pausing"
      pause
      EM.add_timer(5) { puts "resuming"; @rec = 0; resume }
    end
  end

  def unbind
    puts "unbind"
  end
end

case ARGV[0]
when 'popen', nil
  EM.run { EM.popen('cat /dev/random', PopenConnection) }
when 'socket'
  puts "starting netcat on 7777"
  pid = fork { exec "cat /dev/random | nc -l 7777" }
  sleep 1
  EM.run { EM.connect("localhost", 7777, PopenConnection) }
  Process.wait(pid)
end

When run with popen, you can see the pause kick in but then receive_data is called over and over again until the program is interrupted:

$ ruby em-popen-pause-bug.rb popen
receive_data: 8192
receive_data: 8192
<repeats>
receive_data: 8192
pausing
receive_data: 8192
pausing
receive_data: 8192
pausing
receive_data: 8192
pausing
...

The second variation starts a server on 7777 by forking off netcat and then connects to that instead of using popen. There you can see pause and resume working as expected:

$ ruby em-popen-pause-bug.rb socket
starting netcat on 7777
receive_data: 10240
receive_data: 16384
<snip>
receive_data: 1024
pausing
resuming
receive_data: 16384
receive_data: 16384
<snip>
pausing
...
@rtomayko

Looks like the problem is PipeDescriptor overriding SelectForRead() to always return true:

https://github.com/eventmachine/eventmachine/blob/v0.12.10/ext/pipe.cpp#L277-301

This appears to be where the pause logic happens in the ConnectionDescriptor base class:

https://github.com/eventmachine/eventmachine/blob/v0.12.10/ext/ed.cpp#L585

I'm still not sure why paused? is always false.

@tmm1

Looks like c2991c8 added pause/resume only to ConnectionDescriptor, so PipeDescriptor doesn't support it.

This logic should probably go into the EventableDescriptor base class instead.

@rtomayko rtomayko added a commit that referenced this issue Sep 1, 2011
@tmm1 tmm1 pull pause/resume logic down into EventableDescriptor
Fixes pause/resume support on PipeDescriptor and possibly other
connection types. See #245 for more info.
fcaa1a3
@tmm1 tmm1 closed this Sep 29, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment