genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.08k stars 254 forks source link

posix: add support for <sig ../> config nodes #4984

Open Time0o opened 1 year ago

Time0o commented 1 year ago

This is a suggestion for a new type of libc component config nodes that cause POSIX signals to be generated in certain circumstances (for now when some file changes), the change can be found here.

This idea was motivated by the fact that some ported C applications use POSIX signals to trigger configuration file reloads or similar. E.g. mosquitto reloads configuration files on receiving SIGHUP. To avoid having to add patches when porting such applications to Genode, this change allows adding <config> nodes of the following form to libc components:

<config>
    <sig name="SIGHUP" on_file_change="/etc/my_program.config"/>
</config>

This will internally call kill with the component itself as the target when my_program.config is modified.

cproc commented 1 year ago

I think it would be nice if signals could also be sent manually from a shell, for example. Maybe it could be possible to implement this feature with some kind of file system similar to /proc? In this file system each thread could have a signal file and writing a signal number or signal name into the file would make the corresponding component read the signal from the file and handle it? In the mosquitto case a separate component could then watch the config file and write SIGHUP into the signal file of the mosquitto component (assuming that it somehow knows the mosquitto process/thread ID within the file system). Just brainstorming a bit...

chelmuth commented 1 year ago

While the idea sounds quite tempting, I feel uncomfortable with the current implementation that provides an I/O-level mechanism (watcher) that calls a libc API function (kill()). Transferred into a Linux system, this is equivalent to the Linux kernel forcing a thread that is blocked in kernel, calling kill() nested in user land. It would be much better to integrate this feature into libc than flanching it to the entrypoint in the posix shim library.

cproc commented 1 year ago

There's currently some code for emulating SIGWINCH and SIGINT in the libc which might help as reference:

https://github.com/genodelabs/genode/blob/01827959afe8497f659dfce7061c2d33f515a7fa/repos/libports/src/lib/libc/kernel.cc#L298-L331

cproc commented 1 year ago

In principle, the existing SIGWINCH and SIGINT emulation might work with @Time0o's suggested config node mechanism as well:

<sig name="SIGINT" on_file_change="/dev/.terminal/interrupts"/>
<sig name="SIGWINCH" on_file_change="/dev/.terminal/info"/>

And in a scenario with a shell and some kind of /proc file system one could have config nodes like this:

<sig name="SIGHUP" on_file_change="/proc/self/signals/sighup"/>
<sig name="SIGUSR1" on_file_change="/proc/self/signals/sigusr1"/>

and the VFS plugin mounted at /proc could map self to the actual process id in a file system shared with other components. Then the kill command could look for the signal file matching the signal type to be sent in the /proc file system and write anything into it and the component would get notified.

Time0o commented 1 year ago

While the idea sounds quite tempting, I feel uncomfortable with the current implementation that provides an I/O-level mechanism (watcher) that calls a libc API function (kill()). Transferred into a Linux system, this is equivalent to the Linux kernel forcing a thread that is blocked in kernel, calling kill() nested in user land. It would be much better to integrate this feature into libc than flanching it to the entrypoint in the posix shim library.

That is true, I have more or less shoehorned this in because I was not sure whether this would warrant modifying libc. I have just pushed a fixup commit that moves this into kernel.cc. What I haven't done yet is merge this with SIGWINCH/SIGINT handling as proposed by @cproc. Are there more considerations/features that should go into this to make it mergeable?

Time0o commented 1 year ago

@chelmuth, I don't know how to proceed with this issue. Is this feature something you are interested in? If yes, am I on the right track? Should I close this issue if this feature is of no interest?

chelmuth commented 1 year ago

@Time0o the reason for our hesitance to provide a definitive answer is plain lack of time to thoroughly consider the idea at the moment. The libc has become an integral part of many scenarios and use cases. Thus, any change must be designed and implemented with care.

As your implementation seems to fit your requirements and also looks reasonable after skimming your changes, I'd suggest you maintain the feature on your side (best on a public branch referencing this issue). As soon as the requirement for external POSIX signals arises in our use cases, we'll pick up the discussion here. I could imagine SIGHUP could a candidate for such a requirement.

chelmuth commented 3 months ago

With #5305 on the horizon, we may reconsider this feature.