Skip to content

Commit

Permalink
Add Monitoring Thread to Watch for Worker Shutdown
Browse files Browse the repository at this point in the history
Added a thread to the VixDiskLibServer to read one end of a pipe that
the SmartProxyWorker has opened.  When the read returns the worker process
has exited so the VixDiskLibServer shuts down.

This fixes the BZ https://bugzilla.redhat.com/show_bug.cgi?id=1258985

This fix was implemented in lieu of previous PR ManageIQ#4277 -
ManageIQ#4277 which passed SIGTERM
from the worker process to the server process.  The new implementation
will handle any signal that has caused the worker to exit including
SIGKILL. The previous PR will be closed.

Instead of using an environment variable to let VixDiskLibServer know which
FD contains the pipe we are using a known FD.  In addition we have changed
the previous logic for the other pipe being used by these two
processes which is used to hand the DRb URI back to the worker so that it
also uses a known FD.

Finally we have fixed the retry logic in the worker to read the URI before
attempting to establish the DRb connection.

The "known FD" and retry logic code come from PR ManageIQ#4318 by @matthewd which
will be closed in lieu of this new one.
  • Loading branch information
jerryk55 committed Sep 17, 2015
1 parent 9fb4495 commit 010a750
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 32 deletions.
34 changes: 13 additions & 21 deletions gems/pending/VixDiskLib/VixDiskLib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,33 +105,29 @@ def self.start_service
#
# TODO: Get the path to the server programatically - this server should probably live elsewhere.
#
my_env = setup_env
reader, writer = IO.pipe
writerfd = writer.fileno
my_env["WRITER_FD"] = writerfd.to_s

# Ruby 2.0+ automatically closes all descriptors except 0, 1, 2 (input/output/error)
# in child processes. We need the child process (the server) to write it's DRb URI to
# the writer descriptor so the client can read it. Set close_on_exec to false
# in the writer descriptor so it doesn't get closed on us.
# See: https://bugs.ruby-lang.org/issues/5041
writer.close_on_exec = false
my_env = setup_env
uri_reader, uri_writer = IO.pipe
proc_reader, @proc_writer = IO.pipe

server_cmd = "ruby #{SERVER_PATH}/VixDiskLibServer.rb"
$vim_log.info "VixDiskLib.start_service: running command = #{server_cmd}"
pid = Kernel.spawn(my_env, server_cmd,
[:out, :err] => [LOG_FILE, "a"],
:unsetenv_others => true,
:close_others => false,
reader => :close)
writer.close
3 => uri_writer,
4 => proc_reader,
uri_reader => :close,
@proc_writer => :close)
uri_writer.close
proc_reader.close
Process.detach(pid)
$vim_log.info "VixDiskLibServer Process #{pid} started" if $vim_log
DRb.start_service
retry_num = 5
uri = get_uri(uri_reader)
begin
sleep 1
vix_disk_lib_service = DRbObject.new(nil, get_uri(reader))
vix_disk_lib_service = DRbObject.new(nil, uri)
rescue DRb::DRbConnError => e
raise VixDiskLibError, "ERROR: VixDiskLib.connect() got #{e} on DRbObject.new_with_uri()" if retry_num == 0
retry_num -= 1 && retry
Expand All @@ -142,14 +138,10 @@ def self.start_service
def self.get_uri(reader)
if reader.eof
#
# Error - unable to read the port number written into the pipe by the child (Server).
# Error - unable to read the URI with port number written into the pipe by the child (Server).
#
raise VixDiskLibError, "ERROR: VixDiskLib.connect() Unable to determine port used by VixDiskLib Server."
end
uri_selected = reader.gets.split("URI:")
if uri_selected.length != 2
raise VixDiskLibError, "ERROR: VixDiskLib.connect() Unable to determine port used by VixDiskLib Server."
end
uri_selected[1].chomp
reader.gets
end
end
20 changes: 9 additions & 11 deletions gems/pending/VixDiskLib/VixDiskLibServer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ def initialize
@running = nil
end

def writer_to_caller
writer_fd = ENV['WRITER_FD']
writer = IO.new(writer_fd.to_i)
writer
end

def init
VdlWrapper.init
@started = true
Expand Down Expand Up @@ -113,15 +107,19 @@ def wait_for_status(status, secs_to_wait)
#
# Now write the URI used back to the parent (client) process to let it know which port was selected.
#
writer = vddk.writer_to_caller
writer.puts "URI:#{uri_used}"
writer.flush
uri_writer = IO.open(3, 'w')
uri_writer.write "#{uri_used}"
uri_writer.flush
uri_writer.close

proc_reader = IO.open(4, File::RDONLY | File::SYNC)
#
# Trap Handlers useful for testing and debugging.
#
trap('INT') { vddk.shut_down_service("Interrupt Signal received") && exit }
trap('TERM') { vddk.shut_down_service("Termination Signal received") && exit }
trap('INT') { vddk.shut_down_service("Interrupt Signal received"); exit }
trap('TERM') { vddk.shut_down_service("Termination Signal received"); exit }

Thread.new { $vim_log.info "Monitoring Thread"; proc_reader.read; $vim_log.info "Shutting down VixDiskLibServer - Worker has exited"; exit }
#
# If we haven't been marked as started yet, wait for it.
# We may return immediately because startup (and more) has already happened.
Expand Down

0 comments on commit 010a750

Please sign in to comment.