Rtapi/hal realtime type not always initialized

[BsAtHome]

The new hal.get_realtime_type() is not necessarily initialized when HAL is loaded.

In the uspace case, if the HAL memory segment is initiated by a user-space program, then init_hal_data() will simply set it to uninitialized. In uspace, will hal_lib's rtapi_app_main() ever be called? Not to my knowledge because it is not a kernel module or loaded module, but simply a linked library. In uspace, the shared memory segment is mapped/created when hal_init() is called (i.e. create a component) and that does the call to init_hal_data(). However, it never sets the hal_data->realtime_type member to the actual RT type.

This path was discovered when taking out the mapping of the HAL memory from hal_init() into a hal_lib_init() function. The reason for this is that many halmodule functions, like getting and setting named pins, parameter and signals, do not need a component. They only need a mapping of the HAL segment. Therefore, you can call hal_lib_init() when the module is imported and skip a lot of suddenly needless memory existence tests in all the functions.

@hdiethelm @grandixximo any comments on this?

[hdiethelm]

Yes, this is correct.
https://linuxcnc.org/docs/devel/html/en/man/man3/hal_get_realtime_type.3.html says:
For uspace, this returns REALTIME_TYPE_UNINITIALIZED if rtapi_app is not running.

hal_data->realtime_type is initialized at rtapi_app start in hal_lib.c rtapi_app_main(), so you have the true running realtime type. It is set back to uninitialized on exit. There is no other way I found to initialize this value using the rtapi_app internal checks. And anything else is prone to give the wrong type.

Now rtapi_app does still the annoying autostart / autostop. So rtapi_app is only running when there is at leas one realtime component loaded. I have a branch for manual start/stop but this is not yet ready.

For now, you can check if rtapi_app is running using realtime status. If this returns 0, rtapi_app is running.

[hdiethelm]

To go into the details, it is a bit complex.

init_hal_data() does nothing if the shared memory segment is already initialized. What ever happens first:

• If rtapi_app is started first, it loads hal_lib and runs rtapi_app_main() . This calls init_hal_data(), sets hal_data->realtime_type = REALTIME_TYPE_UNINITIALIZED and then hal_data->realtime_type = rtapi_get_realtime_type() in rtapi_app_main()
• If uspace is started first, this calls init_hal_data() and sets hal_data->realtime_type = REALTIME_TYPE_UNINITIALIZED.
• When rtapi_app is started later, init_hal_data() does nothing but hal_data->realtime_type = rtapi_get_realtime_type() is set.

[grandixximo]

A possible fix, assuming check_rt is extended to also print the raw rtapi_realtime_type_t enum on stdout (the name from #4107 stays for humans, the int for programs):

hal_get_realtime_type() falls back to the privileged rtapi_app check_rt when the shared field is still REALTIME_TYPE_UNINITIALIZED (uspace created the segment, no RT component ever loaded, so rtapi_app_main() never ran), and caches the result. The component itself cannot probe, since unprivileged means SCHED_FIFO is denied and the answer comes back as a false negative, so it has to go through the setuid binary.

--- a/src/hal/hal_lib.c
+++ b/src/hal/hal_lib.c
@@
+/* Ask the privileged rtapi_app to detect the realtime type, for the uspace
+ * case where no RT component ever loaded so rtapi_app_main() never populated
+ * the shared field.  Assumes 'check_rt' prints the raw enum on stdout. */
+static hal_realtime_type_t hal_query_realtime_type(void) {
+    FILE *fp = popen("rtapi_app check_rt", "r");
+    if (!fp) return REALTIME_TYPE_UNINITIALIZED;
+    int type = REALTIME_TYPE_UNINITIALIZED;
+    int got = fscanf(fp, "%d", &type);
+    pclose(fp);
+    if (got != 1) return REALTIME_TYPE_UNINITIALIZED;
+    return (hal_realtime_type_t)type;
+}
+
 hal_realtime_type_t hal_get_realtime_type() {
     if (hal_data == 0) {
         rtapi_print_msg(RTAPI_MSG_ERR,
             "HAL: ERROR: hal_get_realtime_type called before init\n");
         return -EINVAL;
     }
+    if (hal_data->realtime_type == REALTIME_TYPE_UNINITIALIZED) {
+        hal_data->realtime_type = hal_query_realtime_type();
+    }
     return hal_data->realtime_type;
 }

Open question: popen forks a shell inside a getter. This path is uspace-component only and never called from RT, so it is not a correctness issue, but execvp with a pipe would avoid the shell if preferred.

Does this look like the right direction, and are you happy to have check_rt print the raw enum alongside the name?

[BsAtHome]

Using a fork/shell/exec in hal_lib is very bad IMO.

[BsAtHome]

Is rtapi_app_main() in hal_lib always called when rtapi_app starts for the first time? It has to be in the forked off master process that encapsulates all RT... Where is that call done?

[hdiethelm]

Using a fork/shell/exec in hal_lib is very bad IMO.

Yes, bad idea. A workaround instead of a fix. But a fix is not that simple.

Is rtapi_app_main() in hal_lib always called when rtapi_app starts for the first time? It has to be in the forked off master process that encapsulates all RT... Where is that call done?

Yes. But there is no fork inside rtapi_app. halcmd fork/exec rtapi_app and it stays running or also not, depending if there is a rt component active or not. The path when rtapi_app is autostarted with a loadrt command:

1. linuxcnc/src/hal/utils/halcmd_commands.cc

   Line 1161 in 1db4ebe

   argv[m++] = EMC2_BIN_DIR "/rtapi_app";

2. linuxcnc/src/hal/utils/halcmd_commands.cc

   Line 1601 in 1db4ebe

   pid = hal_systemv_nowait(argv);

3. linuxcnc/src/hal/utils/halcmd.c

   Line 200 in 1db4ebe

   pid = fork();

4. linuxcnc/src/rtapi/uspace_rtapi_main.cc

   Line 866 in 1db4ebe

   do_load_cmd("hal_lib", std::vector<std::string>());

5. linuxcnc/src/rtapi/uspace_rtapi_main.cc

   Line 468 in 1db4ebe

   int (*start)(void) = DLSYM<int (*)(void)>(module, "rtapi_app_main");

It's hard to warp your head around all this interconnected exe / dll calls.

I rather would do the following:

1. Drop the autostart / autostop: Beside the above issue, this also creates issues where the order of commands in the .hal file matter. I have a working branch, I can create a PR soon. Manual start is a non-issue due to this is anyway always needed with RTAI, so all code is in place to start and stop correctly.
2. Drop the rtapi_app master/client in one exe and create a client library + client exe. So you can call rtapi_app the proper way. No need to always exec rtapi_app from halcmd, just send the request over the socket using the client library. This will be an other probably quite complex undertaking. I am shure I will run in to some annoying road blockers, as so often lately.

[grandixximo]

Agreed, dropping the fork proposal. Your 1 and 2 sound objectively better.

About the hal_lib_init() refactor, as far as I understand, the pin/param/signal functions you want to enable without a component do not read realtime_type, so the uninitialized field is harmless there. It only matters to a caller that actually asks for the type, and for those REALTIME_TYPE_UNINITIALIZED is the honest "no master, no realtime running" answer anyway, so nothing extra is needed for the mapping-without-component case.

[BsAtHome]

About the hal_lib_init() refactor, as far as I understand, the pin/param/signal functions you want to enable without a component do not read realtime_type, so the uninitialized field is harmless there. It only matters to a caller that actually asks for the type, and for those REALTIME_TYPE_UNINITIALIZED is the honest "no master, no realtime running" answer anyway, so nothing extra is needed for the mapping-without-component case.

That is very true. I was just wondering when I looked through the different code paths.

It occurred to me when refactoring hal_lib, removing access to its internals and adding a proper generic query API for userland. The halmodule/python test I was writing didn't need any complex HAL stuff, just access to the shared segment. This is a case where RT is actually active (a thread must be run), but you don't really care about whether it being actual realtime. However, it did expose another deficiency in how to prevent RT/non-RT boundary races without resorting to non-RT sleep() constructs that may or may not work properly (inherent race).

Is rtapi_app_main() in hal_lib always called when rtapi_app starts for the first time? It has to be in the forked off master process that encapsulates all RT... Where is that call done?

Yes. But there is no fork inside rtapi_app. halcmd fork/exec rtapi_app and it stays running or also not, depending if there is a rt component active or not. The path when rtapi_app is autostarted with a loadrt command:

Right, missed that. Thanks.

Refactoring rtapi_app would be a good idea. But, as you say, there may be lots of hidden problems that will only surface once you actually start working on it. I do agree that we just should have a standalone RT app that acts as a server and accepts connections from a client. That client can be encapsulated in a library and then it will be a reusable interface.

[hdiethelm]

So, first PR:
#4206

With this one, manual start / stop is integrated. After that, rtapi_app can be separated into a master / client and a library can be created.

If desired, I can also try to do everything on one PR but I expect it to take quite some time and be huge.