You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2018/04/13 19:14:14 UTC
[1/2] qpid-proton git commit: PROTON-1830: [ruby] container sometimes
fails to close listener
Repository: qpid-proton
Updated Branches:
refs/heads/master 188ce2806 -> 99564eb3c
PROTON-1830: [ruby] container sometimes fails to close listener
Fix race condition where the container might not close a listener if stopped by
an exception. Was causing this sporadic failure:
ContainerTest#test_handler_raise [/home/travis/build/alanconway/qpid-proton/ruby/tests/test_container.rb:328]:
Expected false to be truthy.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/57a22ca9
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/57a22ca9
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/57a22ca9
Branch: refs/heads/master
Commit: 57a22ca99b90daa4544381fe190a3491388dfbbd
Parents: 188ce28
Author: Alan Conway <ac...@redhat.com>
Authored: Fri Apr 13 14:36:17 2018 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Fri Apr 13 15:01:06 2018 -0400
----------------------------------------------------------------------
ruby/lib/core/container.rb | 33 ++++++++++++++++++++++-----------
ruby/lib/core/listener.rb | 14 ++++++++++----
2 files changed, 32 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/57a22ca9/ruby/lib/core/container.rb
----------------------------------------------------------------------
diff --git a/ruby/lib/core/container.rb b/ruby/lib/core/container.rb
index 7d9f2cb..fb0acdc 100644
--- a/ruby/lib/core/container.rb
+++ b/ruby/lib/core/container.rb
@@ -163,7 +163,7 @@ module Qpid::Proton
not_stopped
l = ListenTask.new(io, handler, self)
add(l)
- l
+ l.listener
end
# Run the container: wait for IO activity, dispatch events to handlers.
@@ -275,8 +275,8 @@ module Qpid::Proton
class ListenTask < Listener
def initialize(io, handler, container)
- super
- @closing = @closed = nil
+ @io, @handler = io, handler
+ @listener = Listener.new(io, container)
env = ENV['PN_TRACE_EVT']
if env && ["true", "1", "yes", "on"].include?(env.downcase)
@log_prefix = "[0x#{object_id.to_s(16)}](PN_LISTENER_"
@@ -286,28 +286,33 @@ module Qpid::Proton
dispatch(:on_open);
end
+ attr_reader :listener
+ def closing?() @listener.instance_variable_get(:@closing); end
+ def condition() @listener.instance_variable_get(:@condition); end
+ def closed?() @io.closed?; end
+
def process
- return if @closed
- unless @closing
+ return if closed?
+ unless closing?
begin
return @io.accept, dispatch(:on_accept)
rescue IO::WaitReadable, Errno::EINTR
- rescue IOError, SystemCallError => e
- close e
+ rescue StandardError => e
+ @listener.close(e)
end
end
ensure
- if @closing
+ if closing?
@io.close rescue nil
- @closed = true
- dispatch(:on_error, @condition) if @condition
+ @listener.instance_variable_set(:@closed, true)
+ dispatch(:on_error, condition) if condition
dispatch(:on_close)
end
end
def can_read?() !finished?; end
def can_write?() false; end
- def finished?() @closed; end
+ def finished?() closed?; end
def dispatch(method, *args)
# TODO aconway 2017-11-27: better logging
@@ -316,6 +321,12 @@ module Qpid::Proton
end
def next_tick() nil; end
+
+ # Close listener and force immediate close of socket
+ def close(e=nil)
+ @listener.close(e)
+ @io.close rescue nil
+ end
end
# Selectable object that can be used to wake IO.select from another thread
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/57a22ca9/ruby/lib/core/listener.rb
----------------------------------------------------------------------
diff --git a/ruby/lib/core/listener.rb b/ruby/lib/core/listener.rb
index 82c711a..f7f7d67 100644
--- a/ruby/lib/core/listener.rb
+++ b/ruby/lib/core/listener.rb
@@ -63,12 +63,14 @@ module Qpid::Proton
# @return [Condition] The error condition if there is one
attr_reader :condition
- # Close the listener
+ # Initiate closing the the listener.
+ # It will not be fully {#closed?} until its {Handler#on_close} is called by the {Container}
# @param error [Condition] Optional error condition.
def close(error=nil)
+ return if closed? || @closing
@closing = true
@condition ||= Condition.convert error
- @io.close_read rescue nil # Cause listener to wake out of IO.select
+ @io.close_read rescue nil # Force Container IO.select to wake with listener readable.
nil
end
@@ -78,11 +80,15 @@ module Qpid::Proton
# Get the IP port used by the listener
def port() to_io.addr[1]; end
+ # True if the listening socket is fully closed
+ def closed?() @io.closed?; end
+
private # Called by {Container}
- def initialize(io, handler, container)
- @io, @handler = io, handler
+ def initialize(io, container)
+ @io = io
@container = container
+ @closing = nil
end
end
end
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[2/2] qpid-proton git commit: PROTON-1830: [ruby] fix bug in
test_container_work_queue_stop
Posted by ac...@apache.org.
PROTON-1830: [ruby] fix bug in test_container_work_queue_stop
Previously was scheduling tasks with a delay, so all tasks had different times.
For test to work reliably, must schedule tasks with the same absolute time.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/99564eb3
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/99564eb3
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/99564eb3
Branch: refs/heads/master
Commit: 99564eb3cbbb700cb59843ecbf1ac08165c826d1
Parents: 57a22ca
Author: Alan Conway <ac...@redhat.com>
Authored: Fri Apr 13 14:51:51 2018 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Fri Apr 13 15:02:17 2018 -0400
----------------------------------------------------------------------
ruby/tests/test_container.rb | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99564eb3/ruby/tests/test_container.rb
----------------------------------------------------------------------
diff --git a/ruby/tests/test_container.rb b/ruby/tests/test_container.rb
index 2c460aa..46ad495 100644
--- a/ruby/tests/test_container.rb
+++ b/ruby/tests/test_container.rb
@@ -349,15 +349,24 @@ class ContainerTest < MiniTest::Test
def test_container_work_queue_stop
q = Queue.new
c = Container.new __method__
- t = Thread.new { c.run }
- [0.1, 0.2, 0.2, 0.2, 1.0].each { |d| c.schedule(d) { q << d } }
- assert_equal 0.1, q.pop
- assert_equal 0.2, q.pop
+ thread = Thread.new { c.run }
+ time = Time.now + 0.01
+ # Mix good scheduled tasks at time and bad tasks scheduled after 10 secs
+ 10.times do
+ c.schedule(time) { q << true }
+ c.schedule(10) { q << false }
+ end
+ assert_same true, q.pop # First task processed, all others at same time are due
+ # Mix in some immediate tasks
+ 5.times do
+ c.work_queue.add { q << true } # Immediate
+ c.schedule(time) { q << true }
+ c.schedule(10) { q << false }
+ end
c.stop
- t.join
- assert_equal 0.2, q.pop
- assert_equal 0.2, q.pop
- assert_empty q
+ thread.join
+ 19.times { assert_same true, q.pop }
+ assert_equal 0, q.size # Tasks with 10 sec delay should be dropped
end
# Chain schedule calls from other schedule calls
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org