Github/littlefs
1
Fork 0
Code Actions Activity

Fixed inotify race conditions and fd leak in scripts

Browse Source 
Since we were only registering our inotify reader after the previous
operation completed, it was easy to miss modifications that happened
faster than our scripts. Since our scripts are in Python, this happened
quite often and made it hard to trust the current state of scripts
with --keep-open, sort of defeating the purpose of --keep-open...

I think previously this race condition wasn't avoided because of the
potential to loop indefinitely if --keep-open referenced a file that the
script itself modified, but it's up to the user to avoid this if it is
an issue.

---

Also while fixing this, I noticed our use of the inotify_simple library
was leaking file descriptors everywhere! I just wasn't closing any
inotify objects at all. A bit concerning since scripts with --keep-open
can be quite long lived...
This commit is contained in:
Christopher Haster
2023-11-29 13:14:05 -06:00
parent a9772d785a
commit b8d3a5ef46
2 changed files with 85 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/plot.py
+34 -26
View File
@@ -140,30 +140,32 @@ def openio(path, mode='r', buffering=-1):
else: else:
return open(path, mode, buffering) return open(path, mode, buffering)
def inotifywait(paths): if inotify_simple is None:
# wait for interesting events Inotify = None
inotify = inotify_simple.INotify() else:
flags = (inotify_simple.flags.ATTRIB class Inotify(inotify_simple.INotify):
| inotify_simple.flags.CREATE def __init__(self, paths):
| inotify_simple.flags.DELETE super().__init__()
| inotify_simple.flags.DELETE_SELF
| inotify_simple.flags.MODIFY
| inotify_simple.flags.MOVED_FROM
| inotify_simple.flags.MOVED_TO
| inotify_simple.flags.MOVE_SELF)
# recurse into directories # wait for interesting events
for path in paths: flags = (inotify_simple.flags.ATTRIB
if os.path.isdir(path): | inotify_simple.flags.CREATE
for dir, _, files in os.walk(path): | inotify_simple.flags.DELETE
inotify.add_watch(dir, flags) | inotify_simple.flags.DELETE_SELF
for f in files: | inotify_simple.flags.MODIFY
inotify.add_watch(os.path.join(dir, f), flags) | inotify_simple.flags.MOVED_FROM
else: | inotify_simple.flags.MOVED_TO
inotify.add_watch(path, flags) | inotify_simple.flags.MOVE_SELF)
# wait for event # recurse into directories
inotify.read() for path in paths:
if os.path.isdir(path):
for dir, _, files in os.walk(path):
self.add_watch(dir, flags)
for f in files:
self.add_watch(os.path.join(dir, f), flags)
else:
self.add_watch(path, flags)
class LinesIO: class LinesIO:
def __init__(self, maxlen=None): def __init__(self, maxlen=None):
@@ -1392,6 +1394,11 @@ def main(csv_paths, *,
if keep_open: if keep_open:
try: try:
while True: while True:
# register inotify before running the command, this avoids
# modification race conditions
if keep_open and Inotify:
inotify = Inotify(csv_paths)
if cat: if cat:
draw(sys.stdout) draw(sys.stdout)
else: else:
@@ -1400,11 +1407,12 @@ def main(csv_paths, *,
ring.draw() ring.draw()
# try to inotifywait # try to inotifywait
if inotify_simple is not None: if keep_open and Inotify:
ptime = time.time() ptime = time.time()
inotifywait(csv_paths) inotify.read()
# sleep for a minimum amount of time, this helps issues inotify.close()
# around rapidly updating files # sleep for a minimum amount of time, this helps reduce
# flicker issues
time.sleep(max(0, (sleep or 0.01) - (time.time()-ptime))) time.sleep(max(0, (sleep or 0.01) - (time.time()-ptime)))
else: else:
time.sleep(sleep or 0.1) time.sleep(sleep or 0.1)
scripts/watch.py
+51 -39
View File
@@ -40,30 +40,32 @@ def openio(path, mode='r', buffering=-1):
else: else:
return open(path, mode, buffering) return open(path, mode, buffering)
def inotifywait(paths): if inotify_simple is None:
# wait for interesting events Inotify = None
inotify = inotify_simple.INotify() else:
flags = (inotify_simple.flags.ATTRIB class Inotify(inotify_simple.INotify):
| inotify_simple.flags.CREATE def __init__(self, paths):
| inotify_simple.flags.DELETE super().__init__()
| inotify_simple.flags.DELETE_SELF
| inotify_simple.flags.MODIFY
| inotify_simple.flags.MOVED_FROM
| inotify_simple.flags.MOVED_TO
| inotify_simple.flags.MOVE_SELF)
# recurse into directories # wait for interesting events
for path in paths: flags = (inotify_simple.flags.ATTRIB
if os.path.isdir(path): | inotify_simple.flags.CREATE
for dir, _, files in os.walk(path): | inotify_simple.flags.DELETE
inotify.add_watch(dir, flags) | inotify_simple.flags.DELETE_SELF
for f in files: | inotify_simple.flags.MODIFY
inotify.add_watch(os.path.join(dir, f), flags) | inotify_simple.flags.MOVED_FROM
else: | inotify_simple.flags.MOVED_TO
inotify.add_watch(path, flags) | inotify_simple.flags.MOVE_SELF)
# wait for event # recurse into directories
inotify.read() for path in paths:
if os.path.isdir(path):
for dir, _, files in os.walk(path):
self.add_watch(dir, flags)
for f in files:
self.add_watch(os.path.join(dir, f), flags)
else:
self.add_watch(path, flags)
class LinesIO: class LinesIO:
def __init__(self, maxlen=None): def __init__(self, maxlen=None):
@@ -147,6 +149,21 @@ def main(command, *,
if keep_open_paths and not keep_open: if keep_open_paths and not keep_open:
keep_open = True keep_open = True
# figure out the keep_open paths
if keep_open and inotify_simple is not None:
if keep_open_paths:
keep_open_paths = set(keep_open_paths)
else:
# guess inotify paths from command
keep_open_paths = set()
for p in command:
for p in {
p,
re.sub('^-.', '', p),
re.sub('^--[^=]+=', '', p)}:
if p and os.path.exists(p):
paths.add(p)
returncode = 0 returncode = 0
try: try:
while True: while True:
@@ -157,6 +174,11 @@ def main(command, *,
ring = LinesIO(lines) ring = LinesIO(lines)
try: try:
# register inotify before running the command, this avoids
# modification race conditions
if keep_open and Inotify:
inotify = Inotify(keep_open_paths)
# run the command under a pseudoterminal # run the command under a pseudoterminal
mpty, spty = pty.openpty() mpty, spty = pty.openpty()
@@ -204,24 +226,14 @@ def main(command, *,
pass pass
# try to inotifywait # try to inotifywait
if keep_open and inotify_simple is not None: if keep_open and Inotify:
if keep_open_paths:
paths = set(keep_open_paths)
else:
# guess inotify paths from command
paths = set()
for p in command:
for p in {
p,
re.sub('^-.', '', p),
re.sub('^--[^=]+=', '', p)}:
if p and os.path.exists(p):
paths.add(p)
ptime = time.time() ptime = time.time()
inotifywait(paths) inotify.read()
# sleep for a minimum amount of time, this helps issues around inotify.close()
# rapidly updating files # sleep for a minimum amount of time, this helps reduce
time.sleep(max(0, (sleep or 0.1) - (time.time()-ptime))) # flicker issues
time.sleep(max(0, (sleep or 0.01) - (time.time()-ptime)))
# or sleep
else: else:
time.sleep(sleep or 0.1) time.sleep(sleep or 0.1)
except KeyboardInterrupt: except KeyboardInterrupt: