Repository navigation
Handle invalid poll descriptors in device loop - #88
JeffreyBenjaminBrown wants to merge 1 commit into
Conversation
|
Thanks for the PR! It seems to address multiple concerns though and, since I cannot reproduce the problem locally (on a couple of distros with both FTDI and CDC grids), I wonder which specific descriptor flag check (or lack of) on which descriptor is causing the problem in the first place? |
|
@JeffreyBenjaminBrown while revisiting this today, I've observed 0x19 state on If you're getting POLLNVAL on grid disconnect with your setup, I'd be happy to merge a version of this change, but I cannot see how the part of it checking |
|
Hi Artem. I'm sorry this response took so long. I vibe-coded those two PRs, didn't know what I was doing, and was in over my head. You seem to have been right that the logic was flawed. I'm closing the libmonome patch I submitted as irrelevant, and (below) offering what I think is a good fix. The problem has vanished on my system -- I can unplug a monome without causing serialosc activity to ramp up and stay at 100% of a CPU until closing it. (The old fix seemed to solve it but then it came back.) Even the new fix is something I don't understand, but I did my best to get the AI (Codex) to provide a succinct description and justification. (I tried to edit this PR in the browser, but Github's interface is complex and this is just a four-line change, so instead I'm simply pasting the diff below.) Thanks for the time you spent on this issue, and for your open source work more generally. The rest of this message is by the AI. The revised patchdiff --git a/wscript b/wscript
--- a/wscript
+++ b/wscript
@@
code = """
#include <stdlib.h>
+ #include <fcntl.h>
#include <poll.h>
diff --git a/src/serialosc-device/event_loop/select.c b/src/serialosc-device/event_loop/select.c
--- a/src/serialosc-device/event_loop/select.c
+++ b/src/serialosc-device/event_loop/select.c
@@
- if (FD_ISSET(monome_fd, &rfds))
- monome_event_handle_next(state->monome);
+ if (FD_ISSET(monome_fd, &rfds)
+ && monome_event_handle_next(state->monome) < 0)
+ break;Reasoningserialosc's configure test calls On Linux, unplugging the grid made the tty readable at EOF, but did not mark it The first change makes the configure test valid, restoring the intended poll Live verificationBeforeWith a monome 256 connected at The build log also showed the erroneous fallback: AfterWith the patch applied, configuration reported: The build compiled |
|
ok, so there are 2 problems here. one is that the second problem is that i'm going to manually add missing includes for please feel free to reopen the issue if the problem persists, thanks for getting back to us with the results! |
Fixes a possible CPU spin when poll reports POLLNVAL after a device or IPC fd becomes invalid. This may address #81. (Together with a parallel PR I am submitting now to libmonome, it reduced CPU usage from around 45% to negligible on my system, which can be inspected here: https://github.com/JeffreyBenjaminBrown/nixos-experiments/tree/xfce/config.)