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 2017/10/06 12:51:28 UTC
[1/2] qpid-proton git commit: PROTON-1602: ruby: Possible memory leak
in Ruby bindings
Repository: qpid-proton
Updated Branches:
refs/heads/master 446ade386 -> ace1970ef
PROTON-1602: ruby: Possible memory leak in Ruby bindings
Restore Container timeout to avoid excessive CPU and memory use
Fixed test_container tests to be single-threaded.
Ruby library is not yet ready for multi-threaded use, support for
concurrency is in progress.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/ace1970e
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/ace1970e
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/ace1970e
Branch: refs/heads/master
Commit: ace1970efaea2b5f298acedc62d6838bc424f217
Parents: 121c2d2
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Oct 5 21:00:54 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Fri Oct 6 08:51:13 2017 -0400
----------------------------------------------------------------------
proton-c/bindings/ruby/lib/reactor/reactor.rb | 1 +
proton-c/bindings/ruby/tests/test_container.rb | 167 ++++++++------------
proton-c/bindings/ruby/tests/test_tools.rb | 86 ++--------
3 files changed, 85 insertions(+), 169 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ace1970e/proton-c/bindings/ruby/lib/reactor/reactor.rb
----------------------------------------------------------------------
diff --git a/proton-c/bindings/ruby/lib/reactor/reactor.rb b/proton-c/bindings/ruby/lib/reactor/reactor.rb
index f612876..a84a716 100644
--- a/proton-c/bindings/ruby/lib/reactor/reactor.rb
+++ b/proton-c/bindings/ruby/lib/reactor/reactor.rb
@@ -115,6 +115,7 @@ module Qpid::Proton::Reactor
end
def run(&block)
+ self.timeout = 3.14159265359
self.start
while self.process do
if block_given?
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ace1970e/proton-c/bindings/ruby/tests/test_container.rb
----------------------------------------------------------------------
diff --git a/proton-c/bindings/ruby/tests/test_container.rb b/proton-c/bindings/ruby/tests/test_container.rb
index d5b5c9a..9c1d46c 100644
--- a/proton-c/bindings/ruby/tests/test_container.rb
+++ b/proton-c/bindings/ruby/tests/test_container.rb
@@ -26,78 +26,73 @@ URL = Qpid::Proton::URL
class ContainerTest < Minitest::Test
- # Send n messages
- class SendMessageClient < TestHandler
- attr_reader :accepted
-
- def initialize(url, link_name, body)
- super()
- @url, @link_name, @message = url, link_name, Message.new(body)
- end
+ def test_simple()
- def on_start(event)
- event.container.create_sender(@url, {:name => @link_name})
- end
+ hc = Class.new(TestServer) do
+ attr_reader :accepted
- def on_sendable(event)
- if event.sender.credit > 0
- event.sender.send(@message)
+ def on_start(event)
+ super
+ event.container.create_sender("amqp://#{addr}", {:name => "testlink"})
end
- end
- def on_accepted(event)
- @accepted = event
- event.connection.close
- end
- end
+ def on_sendable(event)
+ if @sent.nil? && event.sender.credit > 0
+ event.sender.send(Message.new("testmessage"))
+ @sent = true
+ end
+ end
- def test_simple()
- TestServer.new.run do |s|
- lname = "test-link"
- body = "hello"
- c = SendMessageClient.new(s.addr, lname, body).run
- assert_instance_of(Qpid::Proton::Event::Event, c.accepted)
- assert_equal(lname, s.links.pop(true).name)
- assert_equal(body, s.messages.pop(true).body)
+ def on_accepted(event)
+ @accepted = event
+ event.container.stop
+ end
end
+ h = hc.new
+ Container.new(h).run
+ assert_instance_of(Qpid::Proton::Event::Event, h.accepted)
+ assert_equal "testlink", h.links.first.name
+ assert_equal "testmessage", h.messages.first.body
end
-
end
class ContainerSASLTest < Minitest::Test
- # Connect to URL using mechanisms and insecure to configure the transport
- class SASLClient < TestHandler
+ # Handler for test client/server that sets up server and client SASL options
+ class SASLHandler < TestServer
- def initialize(url, opts={})
- super()
- @url, @opts = url, opts
- end
+ attr_accessor :url
- def on_start(event)
- event.container.connect(@url, @opts)
+ def initialize(opts={}, mechanisms=nil, insecure=nil, realm=nil)
+ super()
+ @opts, @mechanisms, @insecure, @realm = opts, mechanisms, insecure, realm
end
- def on_connection_opened(event)
+ def on_start(e)
super
- event.container.stop
- end
- end
-
- # Server with SASL settings
- class SASLServer < TestServer
- def initialize(mechanisms=nil, insecure=nil, realm=nil)
- super()
- @mechanisms, @insecure, @realm = mechanisms, insecure, realm
+ @client = e.container.connect(@url || "amqp://#{addr}", @opts)
+ end
+
+ def on_connection_bound(e)
+ if e.connection != @client # Incoming server connection
+ @listener.close
+ sasl = e.transport.sasl
+ sasl.allow_insecure_mechs = @insecure unless @insecure.nil?
+ sasl.allowed_mechs = @mechanisms unless @mechanisms.nil?
+ # TODO aconway 2017-08-16: need `sasl.realm(@realm)` here for non-default realms.
+ # That reqiures pn_sasl_set_realm() at the C layer - the realm should
+ # be passed to cyrus_sasl_init_server()
+ end
end
- def on_connection_bound(event)
- sasl = event.transport.sasl
- sasl.allow_insecure_mechs = @insecure unless @insecure.nil?
- sasl.allowed_mechs = @mechanisms unless @mechanisms.nil?
- # TODO aconway 2017-08-16: need `sasl.realm(@realm)` here for non-default realms.
- # That reqiures pn_sasl_set_realm() at the C layer - the realm should
- # be passed to cyrus_sasl_init_server()
+ attr_reader :auth_user
+ def on_connection_opened(e)
+ super
+ if e.connection == @client
+ e.connection.close
+ else
+ @auth_user = e.transport.sasl.user
+ end
end
end
@@ -119,7 +114,7 @@ class ContainerSASLTest < Minitest::Test
f.write("
sasldb_path: #{database}
mech_list: EXTERNAL DIGEST-MD5 SCRAM-SHA-1 CRAM-MD5 PLAIN ANONYMOUS
-")
+ ")
end
# Tell proton library to use the new configuration
SASL.config_path(conf_dir)
@@ -140,62 +135,36 @@ mech_list: EXTERNAL DIGEST-MD5 SCRAM-SHA-1 CRAM-MD5 PLAIN ANONYMOUS
end
def test_sasl_anonymous()
- SASLServer.new("ANONYMOUS").run do |s|
- c = SASLClient.new(s.addr, {:sasl_allowed_mechs => "ANONYMOUS"}).run
- refute_empty(c.connections)
- refute_empty(s.connections)
- assert_nil(s.connections.pop(true).user)
- end
+ s = SASLHandler.new({:sasl_allowed_mechs => "ANONYMOUS"}, "ANONYMOUS")
+ Container.new(s).run
+ assert_nil(s.connections[0].user)
end
def test_sasl_plain_url()
# Use default realm with URL, should authenticate with "default_password"
- SASLServer.new("PLAIN", true).run do |s|
- c = SASLClient.new("amqp://user:default_password@#{s.addr}",
- {:sasl_allowed_mechs => "PLAIN", :sasl_allow_insecure_mechs => true}).run
- refute_empty(c.connections)
- refute_empty(s.connections)
- sc = s.connections.pop(true)
- assert_equal("user", sc.transport.sasl.user)
- end
+ s = SASLHandler.new({:sasl_allowed_mechs => "PLAIN", :sasl_allow_insecure_mechs => true}, "PLAIN", true)
+ s.url = ("amqp://user:default_password@#{s.addr}")
+ Container.new(s).run
+ assert_equal(2, s.connections.size)
+ assert_equal("user", s.auth_user)
end
def test_sasl_plain_options()
# Use default realm with connection options, should authenticate with "default_password"
- SASLServer.new("PLAIN", true).run do |s|
- c = SASLClient.new(s.addr,
- {:user => "user", :password => "default_password",
- :sasl_allowed_mechs => "PLAIN", :sasl_allow_insecure_mechs => true}).run
- refute_empty(c.connections)
- refute_empty(s.connections)
- sc = s.connections.pop(true)
- assert_equal("user", sc.transport.sasl.user)
- end
- end
-
- # Test disabled, see on_connection_bound - missing realm support in proton C.
- def TODO_test_sasl_plain_realm()
- # Use the non-default proton realm on the server, should authenticate with "password"
- SASLServer.new("PLAIN", true, "proton").run do |s|
- c = SASLClient.new("amqp://user:password@#{s.addr}",
- {:sasl_allowed_mechs => "PLAIN", :sasl_allow_insecure_mechs => true}).run
- refute_empty(c.connections)
- refute_empty(s.connections)
- sc = s.connections.pop(true)
- assert_equal("user", sc.transport.sasl.user)
- end
+ opts = {:sasl_allowed_mechs => "PLAIN",:sasl_allow_insecure_mechs => true,
+ :user => 'user', :password => 'default_password' }
+ s = SASLHandler.new(opts, "PLAIN", true)
+ Container.new(s).run
+ assert_equal(2, s.connections.size)
+ assert_equal("user", s.auth_user)
end
# Ensure we don't allow PLAIN if allow_insecure_mechs = true is not explicitly set
def test_disallow_insecure()
# Don't set allow_insecure_mechs, but try to use PLAIN
- SASLServer.new("PLAIN", nil).run(true) do |s|
- begin
- SASLClient.new("amqp://user:password@#{s.addr}",
- {:sasl_allowed_mechs => "PLAIN", :sasl_allow_insecure_mechs => true}).run
- rescue TestError => e
- assert_match(/PN_TRANSPORT_ERROR.*unauthorized-access/, e.to_s)
- end
- end
+ s = SASLHandler.new({:sasl_allowed_mechs => "PLAIN", :sasl_allow_insecure_mechs => true}, "PLAIN")
+ s.url = "amqp://user:password@#{s.addr}"
+ e = assert_raises(TestError) { Container.new(s).run }
+ assert_match(/PN_TRANSPORT_ERROR.*unauthorized-access/, e.to_s)
end
end
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ace1970e/proton-c/bindings/ruby/tests/test_tools.rb
----------------------------------------------------------------------
diff --git a/proton-c/bindings/ruby/tests/test_tools.rb b/proton-c/bindings/ruby/tests/test_tools.rb
index 2d8f3c6..e9045ee 100644
--- a/proton-c/bindings/ruby/tests/test_tools.rb
+++ b/proton-c/bindings/ruby/tests/test_tools.rb
@@ -49,9 +49,7 @@ class TestPort
end
end
- def close
- @sock.close()
- end
+ def close() @sock.close(); end
end
class TestError < Exception; end
@@ -69,27 +67,17 @@ def wait_port(port, timeout=5)
end
end
-# Handler that creates its own container to run itself, and records some common
-# events that are checked by tests
+# Handler that records some common events that are checked by tests
class TestHandler < MessagingHandler
- # Record errors and successfully opened endpoints
attr_reader :errors, :connections, :sessions, :links, :messages
# Pass optional extra handlers and options to the Container
- def initialize(handlers=[], options={})
+ # @param raise_errors if true raise an exception for error events, if false, store them in #errors
+ def initialize(raise_errors=true)
super()
- # Use Queue so the values can be extracted in a thread-safe way during or after a test.
- @errors, @connections, @sessions, @links, @messages = (1..5).collect { Queue.new }
- @container = Container.new([self]+handlers, options)
- end
-
- # Run the handlers container, return self.
- # Raise an exception for server errors unless no_raise is true.
- def run(no_raise=false)
- @container.run
- raise_errors unless no_raise
- self
+ @raise_errors = raise_errors
+ @errors, @connections, @sessions, @links, @messages = 5.times.collect { [] }
end
# If the handler has errors, raise a TestError with all the error text
@@ -99,13 +87,13 @@ class TestHandler < MessagingHandler
while @errors.size > 0
text << @errors.pop + "\n"
end
- raise TestError.new("TestServer has errors:\n #{text}")
+ raise TestError.new("TestHandler has errors:\n #{text}")
end
# TODO aconway 2017-08-15: implement in MessagingHandler
def on_error(event, endpoint)
@errors.push "#{event.type}: #{endpoint.condition.name}: #{endpoint.condition.description}"
- raise_errors
+ raise_errors if @raise_errors
end
def on_transport_error(event)
@@ -146,61 +134,19 @@ class TestHandler < MessagingHandler
end
end
-# A TestHandler that runs itself in a thread and listens on a TestPort
+# A TestHandler that listens on a TestPort
class TestServer < TestHandler
- attr_reader :host, :port, :addr
-
- # Pass optional handlers, options to the container
- def initialize(handlers=[], options={})
+ def initialize
super
@tp = TestPort.new
- @host, @port, @addr = @tp.host, @tp.port, @tp.addr
- @listening = false
- @ready = Queue.new
- end
-
- # Start server thread
- def start(no_raise=false)
- @thread = Thread.new do
- begin
- @container.listen(addr)
- @container.run
- rescue TestError
- ready.push :error
- rescue => e
- msg = "TestServer run raised: #{e.message}\n#{e.backtrace.join("\n")}"
- @errors << msg
- @ready.push(:error)
- # TODO aconway 2017-08-22: container.stop - doesn't stop the thread.
- end
- end
- raise_errors unless @ready.pop == :listening or no_raise
- end
-
- # Stop server thread
- def stop(no_raise=false)
- @container.stop
- if not @errors.empty?
- @thread.kill
- else
- @thread.join
- end
- @tp.close
- raise_errors unless no_raise
end
- # start(), execute block with self, stop()
- def run(no_raise=false)
- begin
- start(no_raise)
- yield self
- ensure
- stop(no_raise)
- end
- end
+ def host() @tp.host; end
+ def port() @tp.port; end
+ def addr() @tp.addr; end
- def on_start(event)
- @ready.push :listening
- @listening = true
+ def on_start(e)
+ super
+ @listener = e.container.listen(addr)
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: NO-JIRA: ruby: remove dead code
Posted by ac...@apache.org.
NO-JIRA: ruby: remove dead code
unused function do_work
outdated FIXME comment
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/121c2d29
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/121c2d29
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/121c2d29
Branch: refs/heads/master
Commit: 121c2d2927a8ca936729bd31c2f2c17f7b446ebe
Parents: 446ade3
Author: Alan Conway <ac...@redhat.com>
Authored: Fri Sep 29 12:03:58 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Fri Oct 6 08:51:13 2017 -0400
----------------------------------------------------------------------
proton-c/bindings/ruby/lib/reactor/container.rb | 4 ----
proton-c/bindings/ruby/spec/array_spec.rb | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/121c2d29/proton-c/bindings/ruby/lib/reactor/container.rb
----------------------------------------------------------------------
diff --git a/proton-c/bindings/ruby/lib/reactor/container.rb b/proton-c/bindings/ruby/lib/reactor/container.rb
index 4623d7a..2828bb9 100644
--- a/proton-c/bindings/ruby/lib/reactor/container.rb
+++ b/proton-c/bindings/ruby/lib/reactor/container.rb
@@ -238,10 +238,6 @@ module Qpid::Proton::Reactor
end
private
- def do_work(timeout = nil)
- self.timeout = timeout unless timeout.nil?
- self.process
- end
def id(container, remote, local)
if !local.nil? && !remote.nil?
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/121c2d29/proton-c/bindings/ruby/spec/array_spec.rb
----------------------------------------------------------------------
diff --git a/proton-c/bindings/ruby/spec/array_spec.rb b/proton-c/bindings/ruby/spec/array_spec.rb
index b655358..5d91f1a 100644
--- a/proton-c/bindings/ruby/spec/array_spec.rb
+++ b/proton-c/bindings/ruby/spec/array_spec.rb
@@ -17,7 +17,7 @@
# under the License.
#
-require 'spec_helper' #FIXME aconway 2017-09-11:
+require 'spec_helper'
describe "The extended array type" do
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org