-
Notifications
You must be signed in to change notification settings - Fork 25
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
Rewrite of wait_readable to use the Cellulid::ZMQ mailbox reactor #21
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,9 @@ | |
module Celluloid | ||
# Actors which run alongside 0MQ sockets | ||
module ZMQ | ||
class NotZmqActorError < StandardError; end | ||
class NotZmqSocketError < StandardError; end | ||
|
||
class << self | ||
attr_writer :context | ||
|
||
|
@@ -31,9 +34,40 @@ def terminate | |
end | ||
end | ||
|
||
extend Forwardable | ||
def wait_readable(socket) | ||
if !socket.is_a?(::ZMQ::Socket) | ||
throw NotZmqSocketError | ||
end | ||
actor = Thread.current[:celluloid_actor] | ||
if actor && actor.mailbox.is_a?(Celluloid::ZMQ::Mailbox) | ||
actor.mailbox.reactor.wait_readable(socket) | ||
else | ||
throw NotZmqActorError | ||
end | ||
nil | ||
end | ||
module_function :wait_readable | ||
|
||
def wait_writable(socket) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You misspelt It doesn't seem to be used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how you spell things down there in New Zealand, but around here it's "writable" ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have a look around the rest of celluloid-zmq! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Failure to use wait_writable is likely an oversight. The existing occurrences of "writeable" in the codebase are spelling errors. |
||
actor = Thread.current[:celluloid_actor] | ||
if actor && actor.mailbox.is_a?(Celluloid::ZMQ::Mailbox) | ||
actor.mailbox.reactor.wait_writable(socket) | ||
else | ||
Kernel.select([], [io]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This won't work. For one, "io" isn't defined. For two, you can't use Kernel.select on 0MQ sockets |
||
end | ||
nil | ||
end | ||
module_function :wait_writable | ||
|
||
# Does the 0MQ socket support evented operation? | ||
def evented? | ||
actor = Thread.current[:celluloid_actor] | ||
return unless actor | ||
|
||
mailbox = actor.mailbox | ||
mailbox.is_a?(Celluloid::IO::Mailbox) && mailbox.reactor.is_a?(Celluloid::ZMQ::Reactor) | ||
end | ||
module_function :evented? | ||
|
||
# Wait for the given IO object to become readable/writeable | ||
def_delegators 'current_actor.mailbox.reactor', :wait_readable, :wait_writeable | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,15 +38,6 @@ def close | |
@socket.close | ||
end | ||
|
||
# Does the 0MQ socket support evented operation? | ||
def evented? | ||
actor = Thread.current[:celluloid_actor] | ||
return unless actor | ||
|
||
mailbox = actor.mailbox | ||
mailbox.is_a?(Celluloid::IO::Mailbox) && mailbox.reactor.is_a?(Celluloid::ZMQ::Reactor) | ||
end | ||
|
||
# Hide ffi-rzmq internals | ||
alias_method :inspect, :to_s | ||
end | ||
|
@@ -68,7 +59,7 @@ def connect(addr) | |
|
||
# Read a message from the socket | ||
def read(buffer = '') | ||
Celluloid.current_actor.wait_readable(@socket) if evented? | ||
Celluloid::ZMQ.wait_readable(@socket) if Celluloid::ZMQ.evented? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you moved the check inside the method, it would be more closer to celluloid-io. |
||
|
||
unless ::ZMQ::Util.resultcode_ok? @socket.recv_string buffer | ||
raise IOError, "error receiving ZMQ string: #{::ZMQ::Util.error_string}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure we should raise here.
@tarcieri ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably just call it a TypeError or ArgumentError