Skip to content

Commit

Permalink
be more careful around shutdown, make sure events don't trigger a seg…
Browse files Browse the repository at this point in the history
…fault
  • Loading branch information
slyphon committed Jun 16, 2011
1 parent 5597525 commit 9d97bab
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 45 deletions.
23 changes: 12 additions & 11 deletions ext/zookeeper_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ def reopen(timeout = 10, watcher=nil)
set_default_global_watcher(&watcher)
end

init(@host)

# $stderr.puts "@_running is #{@_running.inspect}"

@start_stop_mutex.synchronize do
init(@host)

if timeout > 0
time_to_stop = Time.now + timeout
until state == Zookeeper::ZOO_CONNECTED_STATE
Expand Down Expand Up @@ -85,16 +86,16 @@ def associating?

def close
@start_stop_mutex.synchronize do
if @_running
@_running = false
@_running = false if @_running
end

if @dispatcher
wake_event_loop! unless closed?
@dispatcher.join
end
end
if @dispatcher
wake_event_loop! unless @_closed
@dispatcher.join
end

unless closed?
@start_stop_mutex.synchronize do
unless @_closed
close_handle

# this is set up in the C init method, but it's easier to
Expand All @@ -121,11 +122,11 @@ def set_default_global_watcher(&block)
end

def closed?
false|@_closed
@start_stop_mutex.synchronize { false|@_closed }
end

def running?
false|@_running
@start_stop_mutex.synchronize { false|@_running }
end

protected
Expand Down
36 changes: 15 additions & 21 deletions java/zookeeper_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -430,35 +430,29 @@ def wake_event_loop!

def close
@req_mutex.synchronize do
if @_running
@_running = false

@_running = false if @_running
end

# XXX: why is wake_event_loop! here?
if @dispatcher
wake_event_loop!
@dispatcher.join
end
end
# XXX: why is wake_event_loop! here?
if @dispatcher
wake_event_loop!
@dispatcher.join
end

unless closed?
@event_queue.close
unless @_closed
@start_stop_mutex.synchronize do
@_closed = true
close_handle
end

@event_queue.close
end
end

def close_handle
@req_mutex.synchronize do
return if @_closed
@_closed = true
end

@start_stop_mutex do
if @jzk
@jzk.close
wait_until { !connected? }
end
if @jzk
@jzk.close
wait_until { !connected? }
end
end

Expand Down
10 changes: 5 additions & 5 deletions lib/zookeeper/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ module ZookeeperCommon
# sigh, i guess define this here?
ZKRB_GLOBAL_CB_REQ = -1

def get_next_event(blocking=true)
return nil if closed? # protect against this happening in a callback after close
super(blocking)
end

protected
def setup_call(opts)
req_id = nil
Expand Down Expand Up @@ -34,11 +39,6 @@ def get_completion(req_id)
@req_mutex.synchronize { @completion_reqs.delete(req_id) }
end

def get_next_event(blocking=true)
return nil if closed? # protect against this happening in a callback after close
super(blocking)
end

def dispatch_next_callback(blocking=true)
hash = get_next_event(blocking)
Zookeeper.logger.debug { "get_next_event returned: #{hash.inspect}" }
Expand Down
19 changes: 11 additions & 8 deletions lib/zookeeper/em_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,26 @@ def on_attached(&block)
def close(&block)
on_close(&block)

@start_stop_mutex.synchronize do
logger.debug { "close called, closed? #{closed?} running? #{running?}" }

if running?
if @_running
@start_stop_mutex.synchronize do
@_running = false
end

@em_connection.detach if @em_connection
@em_connection = nil
@em_connection.detach if @em_connection
@em_connection = nil

unless closed?
unless @_closed
@start_stop_mutex.synchronize do
logger.debug { "closing handle" }
close_handle
selectable_io.close unless selectable_io.closed?
end
else
logger.debug { "we are not running, so returning on_close deferred" }

selectable_io.close unless selectable_io.closed?
end
else
logger.debug { "we are not running, so returning on_close deferred" }
end

on_close.succeed
Expand Down

0 comments on commit 9d97bab

Please sign in to comment.