Repository navigation
Gracefully handle too-many-FDs-open/EMFILE + possibly other "should never happen" (but actually might happen tho) spots. #162
Description
Background 1: #161 talks of reducing FD use, and that is great to do; it would lower importance of present ticket among other things. That said, what led to that ticket is we added a test and it happened to unexpectedly hit EMFILE (Linux) which focused us to the high per-object FD use in Flow-IPC. It also focused us to the present deficiency, which is that the various places where EMFILE is reachable tend to assume would never fail unless the system is in total disarray. When it's a problem, it's something like thing failed => assert(false && "This should never happen <more words>."
-
Mitigation: Not all FD-grabbing places do this.
-
Mitigation: As noted in Perf: Lower FD use/avoid too-many-FDs-open: Specific FD savings + survey + possible Native_socket_stream_acceptor internal rejigger. #161, the modern-default limit=1024 is eminently raisable by users and in high-load applications is pretty much always raised, possibly to millions.
-
Mitigation: If logging is enabled, a nice WARNING with all details including the fact EMFILE is what happened is indeed logged.
-
Bad: assert()s may well be off; in which case what happens is undefined-behavior mayhem.
-
Bad: if on, it's a crash, and the assert() msg does not react to EMFILE specifically, so it's not even clear EMFILE happened, unless logging is on, and one checks logs. We Flow-IPC devs often do, but lots of library users would just pass Logger* = null, and that's that.
-
Bad: sometimes the assert(false && "...") is followed by std::abort(), which is at least deterministic in terms of crashing it all to the ground... but if assert()s are off, and logging is off or didn't get flushed or something, then one has no idea what went wrong at all. There's the core file potentially, but you get the point.
To solve: Find all the bad sites; detect EMFILE-likes specifically; emit error (and log about it) as gracefully as possible. E.g., if it occurs during session-log-in -- fail the log-in, emit EMFILE (or whatever it was). Just be cool... be helpful. Generally Flow-IPC is extremely helpful in terms of error reporting and recovery; but for this stuff, for code simplicity, it just isn't. And that's justifiable, if it would really ~never happen in an unborked environment -- but EMFILE is not actually that, we've discovered.
Background 2: EMFILE (or other-OS equivalent; as of this writing targeting Linux only, but it may well change, possibly to non-POSIX a/k/a Windows) is the main target. But ideally we should find all assert()s (grep it and/or use LLM for smarter analysis) that do a native call and then freak-out if it fails in any way. Look into what could actually happen specifically there to make it fail.
If EMFILE is the only realistic thing... then fine. The "Background 1" solution is enough. If not though... be cool and emit/recover gracefully where possible. And hey, possibly, even if EMFILE is the only realistic thing, at that point the cost of emitting whatever-happened/recovering might be ~zero, if EMFILE is handled nicely anyway.
(Parting note here: connect_pipe() utility, in particular, being treated as infallible is not great; this tool is used in several places to hook-up sync_io timers at least; we shouldn't just let it internally assume it can never fail. Probably.)
Priority: Depends on #161; but pretty much by definition do at least the low-hanging-fruit ideas from #161 first. Then consider the priority in here. All in all, low-to-medium.
Activity