threaded non-blocking close - #145

Here is some output that shows that it still closes correctly:

#!/usr/bin/python3
import evdev
import time

def a():
    stuff = [evdev.InputDevice(path) for path in evdev.list_devices()]
    time.sleep(10)
    print('done')
    return stuff

a()
print('sleep')
time.sleep(10)
print('end')

➜  ~ python3 evdev_threaded_close.py &
[2] 13226
➜  ~ ls -l /proc/13226/fd
total 0
lrwx------ 1 mango mango 64 Nov 16 13:10 0 -> /dev/pts/2
lrwx------ 1 mango mango 64 Nov 16 13:10 1 -> /dev/pts/2
lrwx------ 1 mango mango 64 Nov 16 13:10 2 -> /dev/pts/2
lrwx------ 1 mango mango 64 Nov 16 13:10 3 -> /dev/input/event7
lrwx------ 1 mango mango 64 Nov 16 13:10 4 -> /dev/input/event6
lrwx------ 1 mango mango 64 Nov 16 13:10 5 -> /dev/input/event5
➜  ~ done
thread closing 5
thread closing 4
thread closing 3
sleep
➜  ~ ls -l /proc/13226/fd
total 0
lrwx------ 1 mango mango 64 Nov 16 13:10 0 -> /dev/pts/2
lrwx------ 1 mango mango 64 Nov 16 13:10 1 -> /dev/pts/2
lrwx------ 1 mango mango 64 Nov 16 13:10 2 -> /dev/pts/2
lrwx------ 1 mango mango 64 Nov 16 13:10 6 -> 'anon_inode:[eventpoll]'
lrwx------ 1 mango mango 64 Nov 16 13:10 7 -> 'socket:[328870]'
lrwx------ 1 mango mango 64 Nov 16 13:10 8 -> 'socket:[328871]'
➜  ~ end
[2]  + 13226 done       python3 evdev_threaded_close.py

Thanks for debugging this and providing a fix!

I really hope it won't cause any unwanted side effects (race conditions) at some point in other applications. But it does make things easier, I had to put my stuff into a separate process to make sure things run smoothly.

But thinking about it, it seems unlikely. Opening the device creates a new fd so it should be independent of the previous one, even if that one has exclusive access (since grabbing is process-wide, isn't it?)

This patch ended up being the root cause for some weird hanging I was seeing when closing dragonfly; if I revert this back to a simple call to os.close() then it doesn't hang on quit anymore.

It might be something with trying to create a thread while python is shutting down? But also I'm not seeing the blocking effects with unthreaded os.close that @sezanzeb was so this might be something specific to my machine.

• Python 3.9.7
• python-evdev 1.4.0
• Linux 5.14.2-arch1-2

@scauligi Did you look at #144? So you are definitely not having performance issues there when handling lots of InputDevices?

Yeah, I guess the change was quite risky after all. Maybe it should be reverted

Do you know if asyncio supports closing files in a non-blocking way? Is a non-blocking close even theoretically possible with an event loop on a single process?

Hmm yeah the code in #144 takes about a half second between printing and exiting for me, but that's definitely preferable to the indefinite hanging I was getting earlier. If I iterate over the devices in a loop and explicitly call del on each device, then it still takes the same amount of time, but at least it feels more responsive; it looks like it really is just each individual call to close taking a brief amount of time, all adding up.

As far as alternatives... I think the main issue comes from trying to do anything fancy inside of __del__, since you start running into all sorts of edge cases due to garbage collection and the interpreter terminating.
If you're dealing with that many InputDevices, then I guess you could either explicitly call del as needed to amortize the closing cost (kind of gross though) or have an explicit "closer thread" already running that gets passed the devices through a SimpleQueue and closes them, which hopefully will avoid any deadlocks (also pretty gross).

But in either case, (and from looking at what the CPython code does in some of its async stream handling), I think the right call is to leave it as a simple os.close and have the user do any threaded closing.

How about adding a parameter to the InputDevice constructor to change the closing behaviour? Could default to False to use the original os.close method, and True to use threading. threaded_close or something, with a nice docstring to explain why.

I'll quickly draft something and share it here. I'm not sure if that is good design though since it would still be a workaround, idk.

#165

I'm totally fine with reverting it completely if you are also not sure if adding a parameter for this is good design.

In my project it is in a separate process anyway, so it won't be negatively affected by removing the threading

I agree that adding a parameter doesn't feel like a good design, I'd prefer just reverting it. It's probably still a good idea to have text somewhere documenting this behavior though, just so other users are aware going into it.