Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add exception handling around fapd handle is_active() call #560

Merged
merged 15 commits into from Aug 17, 2022
Merged

Add exception handling around fapd handle is_active() call #560

Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
51b240c
Add exception handling around fapd handle is_active() call
tparchambault Jul 26, 2022
a97e3a8
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Jul 26, 2022
941215a
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Jul 27, 2022
df8fc46
Adding additional delay to test if an fapolicyd.active() error may a…
tparchambault Jul 27, 2022
2a837e8
Sync with master
tparchambault Jul 29, 2022
93f59b3
Overwrite my local fapolicyd.rs with latest from master
tparchambault Jul 29, 2022
ff82265
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 1, 2022
b5bede9
More specific exception msg when acquiring first fapd status
tparchambault Aug 1, 2022
167f9ff
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 15, 2022
9cdbbf3
Fix test target command
tparchambault Aug 15, 2022
167d0d9
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 16, 2022
2a10fe0
Moved blocking lock acquisition into context manager blocks
tparchambault Aug 16, 2022
5eee4a2
Merge branch 'master' into fa544_ui_unresponsive_rollback
jw3 Aug 17, 2022
8270b6b
Add toaster error msgs when fapd start/stop exceptions occur
tparchambault Aug 17, 2022
6b01a66
Merge branch 'master' into fa544_ui_unresponsive_rollback
tparchambault Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 26 additions & 14 deletions fapolicy_analyzer/ui/fapd_manager.py
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@

import logging
import os
import sys
import subprocess


Expand Down Expand Up @@ -209,15 +210,16 @@ def _status(self):
logging.debug("fapd is currently DISABLED")
return ServiceStatus.UNKNOWN
elif self.mode == FapdMode.ONLINE:
try:
if self._fapd_lock.acquire(blocking=False):
if self._fapd_lock.acquire(blocking=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use a with block here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into that approach originally. The fly/ointment thing is the blocking=False argument, i.e. if the lock is not available then don't wait and return False. Fapd control operations (start/stop) are given priority over the never-ending periodic status requests.

Still digging around. I would still prefer to have locking somewhere given the status requests and stop/start requests originate on differing threads. I don't care if it's under the hood in rust-land or not.

try:
bStatus = ServiceStatus(self._fapd_ref.is_active())
if bStatus != self._fapd_status:
logging.debug(f"_status({bStatus} updated")
self._fapd_status = bStatus
self._fapd_lock.release()
except Exception:
logging.warning("Daemon monitor query/update dispatch failed.")
except Exception:
logging.warning("Daemon status query/update failed.")

self._fapd_lock.release()
return self._fapd_status
else:
logging.debug("fapd is in a PROFILING session")
Expand All @@ -227,10 +229,15 @@ def _status(self):
return ServiceStatus.FALSE

def capture_online_state(self):
logging.debug("capture_online_state()")
updated_online_state = ServiceStatus(self._fapd_ref.is_active())
self._fapd_prior_online_state = updated_online_state
self._fapd_cur_online_state = updated_online_state
logging.debug("capture_online_state(): Capturing current online status")
if self._fapd_lock.acquire():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, with would be ideal on these locked blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, with context management is Ok here. Will do.

try:
updated_online_state = ServiceStatus(self._fapd_ref.is_active())
self._fapd_prior_online_state = updated_online_state
self._fapd_cur_online_state = updated_online_state
except Exception:
logging.warning("Daemon online status query/update failed.")
self._fapd_lock.release()
logging.debug(f"prior_online_state = {self._fapd_prior_online_state}")

def revert_online_state(self):
Expand All @@ -241,9 +248,14 @@ def revert_online_state(self):

def initial_daemon_status(self):
if self._fapd_lock.acquire():
self._fapd_ref = Handle("fapolicyd")
if self._fapd_ref.is_valid():
self._fapd_status = ServiceStatus(self._fapd_ref.is_active())
else:
self._fapd_status = ServiceStatus.UNKNOWN
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and again.

self._fapd_ref = Handle("fapolicyd")
if self._fapd_ref.is_valid():
self._fapd_status = ServiceStatus(self._fapd_ref.is_active())
else:
self._fapd_status = ServiceStatus.UNKNOWN
except Exception as e:
print(f"Fapd Handle is_valid() or is_active() exception: {e}",
file=sys.stderr)
raise
self._fapd_lock.release()