Skip to content

Commit

Permalink
attempt to catch exceptions from usb_handler child process
Browse files Browse the repository at this point in the history
This commit adds a 'status' instance field to the multiprocessing.Process() child process, which starts-out at 0 and gets incremented by the child process at some unknown interval (actually its updated by the parent by listening to a 'ping' command placed into the queue by the child).

This allows the parent to check-on the state of the child process after launching it, which is important because exceptions thrown by the child won't raise an exception up to the parent (we could catch it, but that would be blocking).

This allows us to detect if there's issues immediately after the user clicks the "arm" button. After this commit, we can prevent the app from giving the user a false sense of security, because we'll throw an error on toggle() if the child isn't well and the GUI will remain blue/disarmed rather than saying it's armed (even if it can't possibly trigger because the child process has issues)

 * #77 (comment)
  • Loading branch information
maltfield committed Jul 27, 2024
1 parent ad12cb2 commit 395cc76
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 7 deletions.
6 changes: 3 additions & 3 deletions build/linux/buildAppImage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ set -x
#
# Authors: Michael Altfield <[email protected]>
# Created: 2020-05-30
# Updated: 2024-03-22
# Version: 1.3
# Updated: 2024-07-26
# Version: 1.4
################################################################################

################################################################################
Expand Down Expand Up @@ -115,7 +115,7 @@ fi
# get the app's dependencies
tmpDir="`mktemp -d`" || exit 1
pushd "${tmpDir}"
git clone https://github.com/BusKill/buskill-app-deps.git
git clone --no-tags --depth 1 https://github.com/BusKill/buskill-app-deps.git

mkdir gnupg
chmod 0700 gnupg
Expand Down
5 changes: 1 addition & 4 deletions src/buskill_gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -1116,10 +1116,7 @@ def rearm_if_required(self):
# the trigger was changed; update the runtime bk instance
self.bk.set_trigger( new_trigger )
except Exception as e:

# TODO: add logic to determine if set_trigger() failed (eg if we were
# unable to launch a root_child process) and: raise GUI error message
# && reset back to the previous trigger
# we got an exception when trying to change the trigger; show in GUI
msg = "Unable to set trigger to '" +str(new_trigger)+ "'"
msg += "\n\n" +str(e)
print( msg )
Expand Down
45 changes: 45 additions & 0 deletions src/packages/buskill/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,37 @@ def toggle(self):
)
self.usb_handler.start()

# did we arm successfully?
while True:
# don't update the UI until the child process either sends us a
# 'ping' or throws an exception

self.check_usb_handler(0)

# TODO: remove prints
print( "usb_handler._exception:|" +str(self.usb_handler._exception)+ "|" )
print( "usb_handler.status:|" +str(self.usb_handler.status)+ "|" )

# did the child process throw an exception?
if self.usb_handler._exception:
# the child process threw an exception; raise it now

msg = "ERROR: child process throw exception:" +str(e)
print( msg ); logger.error( msg )

# raise an error to be caught by the UI
raise ChildProcessError(msg)

# did the child process increment its status?
if self.usb_handler.status > 0:
# the child process appears to be working; continue with changing
# the GUI to indicate that we're armed successfully
break

# TODO: remove sleep, if possible
#import time
#time.sleep(1)

self.is_armed = True
msg = "INFO: BusKill is armed. Listening for removal event.\n"
msg+= "INFO: To disarm the CLI, exit with ^C or close this terminal"
Expand Down Expand Up @@ -1069,6 +1100,12 @@ def check_usb_handler( self, dt ):
# the child told us to execute the trigger; do it!
self.TRIGGER_FUNCTION()
return queue_message

if queue_message == 'ping':
# the child just wants to let us know that it's alive; increment
# the status instance field for the child process
self.usb_handler.status += 1

else:
# no idea what the child said; log it as an error
msg = "ERROR: Unknown queue message from child usb_handler"
Expand Down Expand Up @@ -1112,6 +1149,8 @@ def armNix( self ):

try:
while True:
self.usb_handler_queue.put( 'ping' )

# this call is blocking (with a default timeout of 60 seconds)
# afaik there's no way to tell USBContext.handleEvents() to exit
# safely, so instead we just make the whole call to this arming
Expand Down Expand Up @@ -1152,6 +1191,8 @@ def armWin( self ):
w = Notification( self )
win32gui.PumpMessages()

self.usb_handler_queue.put( 'ping' )

#####################
# TRIGGER FUNCTIONS #
#####################
Expand Down Expand Up @@ -1626,6 +1667,10 @@ def __init__(self, *args, **kwargs):
self._pconn, self._cconn = multiprocessing.Pipe()
self._exception = None

# this is just some number that the child process constantly increments
# it can be used by the parent to check on the status of the child
self.status = 0

def run(self):

try:
Expand Down

0 comments on commit 395cc76

Please sign in to comment.