Draft - HAL types update and HAL isolation

[BsAtHome]

This is the full tree conversion for the HAL types where #4099 would be the first step.
Changes:

• finalize (small minor) details regarding the new HAL types
• retire code that performs direct access to the underlying HAL memory for all HAL types
• covert all HAL components to the new API
• convert all kinematics to the new API
• convert all HAL drivers to the new API
• convert halmodule and other code that used the "old" pin/param access method(s)
• rewrite halrmt such that it is actually functional
• introduce a HAL query API for user-space, including hal_get_p, hal_set_p, hal_get_s and hal_set_s and more. This reduces code duplication and re-implementations that may or may not be fully identical
• make the HAL mutex recursive (for query callback reentry) and only accessible by the HAL library
• convert all code that included hal_priv.h and used direct HAL memory access to use the new query API
• revamp of the hal_*(3) man pages and full documentation of all the new API (old stuff will be removed eventually)

State of progress of these changes:

• the HAL library's internals are now fully private to the library
• the hal_data_u is never used in the code anymore (except for HAL library internal) and is ready to be retired
• the HAL API still retains full compatible with the "old" ways until the API break is performed
• all CI passes, but there may be some missed issues. Every (incremental) change obviously needs critical review before submitted
• waiting to move forward...

Missing in this tree:

• HAL_PORT/hal_port_t changes because they would be a breaking change

[grandixximo]

Never cease to amaze me...
Are we having a meeting with #4099 as central discussion? We skipped a few weeks, not sure why @rmu75
Seems about time... or is everyone on summer vacation? Probably better send to the mailing list...

[rmu75]

On Fr, 2026-07-17 at 07:08 -0700, Luca Toniolo wrote: grandixximo left a comment (LinuxCNC/linuxcnc#4247) Never cease to amaze me... Are we having a meeting with #4099 as central discussion? We skipped a few weeks, not sure why @rmu75 Seems about time... or is everyone on summer vacation? Probably better send to the mailing list...

I had some unexpected troubles with "remote" internet access around 21st of june and it seems I got a bit sidetracked... sorry for that. Last meeting I have notes for was 2026-06-07 10:00 CEST According to our cadence, 2026-06-21 would have been late, 2026-07-05 early and the next meeting would be tomorrow "late" at 20:00 CEST. I'm not sure I can make it this weekend but I will try. I think luca wants to discuss the hal interface redesign, so maybe change of plan and meeting at 10:00 would be better? please reply whoever can make it at what time. https://greenlight.bbb.uni-rostock.de/b/ste-c4d-brs-3k6 Access code: 869782 (hope this is still active) best regards

…

-- Robert Schöftner ***@***.***>

[grandixximo]

Should send to the mailing list, my bad for starting it here 😞

[BsAtHome]

@grandixximo the whole loadusr code needs to be reviewed separately. The constructs in both halcmd and halrmt feel off seem to have a bad smell. I don't think all cases of what can happen are caught correctly in either program. Luckily, it will be some time before this code will make its way. So time to put the thinking cap on and see if it can be made into something smelling fresh and likable ;-)

[grandixximo]

Thanks for the quick turnaround, checked the new push. The unload inversion, PyErr_Format swaps, alias pointers, halquery threads (all three), SET NET, and the ADDF bounds are all correctly fixed. On the legacy-param setter: agreed with your conclusion, diverting hal_set_p to type-width hal_data_u writes for old-style params until the API break closes the hole.

Two Out-of-Bounds of the same family as ADDF remain:

1. DELF: table still declares nset=1 (halrmt.cc:2215) but setDelF reads ctx.toks[3] unconditionally. Same fix as ADDF: nset=2.
2. SET SAVE: error path formats ctx.toks[3] (halrmt.cc:2092) with nset=1, so SET SAVE badkey without a filename reads out of bounds.

Minor: literal %d inside fmt::format strings (e.g. setAddf's error message) prints verbatim and drops rv, there are a few in src/hal/utils/halrmt.cc fix is mechanical, replace %d with {}.

Loadusr: agreed, the whole path wants a proper rewrite, here or in a follow up.

[BsAtHome]

Should be all fixed now, except the param write in hal_set_p, which will take more than a mere moment to alter.

[rene-dev]

I just tested this branch on a machine with smartserial slaves. the driver loads, and creates all the pins, but nothing works. IO are not getting to the hardware. I cannot see any error messages.

[BsAtHome]

I just tested this branch on a machine with smartserial slaves. the driver loads, and creates all the pins, but nothing works. IO are not getting to the hardware. I cannot see any error messages.

Thanks for testing.
At least the pin creation went well and it didn't crash. That is not entirely bad.

But, I haven't run locally on the hardware for some time. Need to check up on that. Also, the changes in hostmot2 have not yet been properly reviewed, so there can be lurking any detail in any layer of the hm2 driver stack. I'll give a peep when this can be tested again.

[grandixximo]

@rene-dev

Does your test config string enable absenc (ssi/biss/fanuc formats)?
What does halcmd show pin hm2_*.sserial.port-0.port_state shows?
Could you grep dmesg for pin_new(...) called with already-initialized memory or Timeout waiting for CMD to clear

[grandixximo]

Did the review below...

[grandixximo]

Checked and not a problem: value_object() has no duplicate-case risk (HAL_SINT/HAL_UINT are unconditionally HAL_S64/HAL_U64 on master now). The piecemeal py::dict style dodges the cppcheck portability warning as commented in pm. The get_info_* semantics in halmodule.cc (list of dicts, same keys) are unchanged, only the collection moved behind the HalQRec snapshot. CI green on a7a528d.

[grandixximo]

halquery.so builds unconditionally. What does a source build on a host without python3-pybind11 do now, given configure never probes for the headers? Debian is covered via debian/control, but elsewhere it looks like a mid-compile missing-header failure rather than a clean configure diagnostic. Worth a configure probe, or a note in the build docs?

[BsAtHome]

Yes, this is a test build so far.
Need to have a configure test to make sure.

[BsAtHome]

Now, the real problem I'm having is that the halquery.cc file is a normal module that has a a PyInit_halquery entry.

I'm currently at a loss how to create only the submodule with pybind11 that can be added in halmodule using a PyModule_AddObject() call (and does it need a PyType_Ready() call too?).

Your version uses a fake module created within the bounds of halmodule. That does not work here, I believe.

[grandixximo]

PYBIND11_MODULE(halquery, m) is only a macro: it emits PyInit_halquery plus a function body that takes a py::module_ &. Nothing else in pybind11 depends on that entry point, so you can write the function yourself and hand it the _hal module object.

In halquery.cc, drop the macro and keep the body:

extern "C" int halquery_add_submodule(PyObject *parent)
{
    try {
        py::module_ p = py::reinterpret_borrow<py::module_>(parent);
        py::module_ m = p.def_submodule("query", halquery_module_doc);

        m.def("pin",   &get_pin,   "Get information about the named pin or None if not found.");
        // ... the rest of the defs verbatim

        halenumtype = p.attr("type");
        halenumdir  = p.attr("dir");
        halenumcomp = p.attr("comptype");
        return 0;
    } catch(const std::exception &e) {
        PyErr_SetString(PyExc_ImportError, e.what());
        return -1;
    }
}

def_submodule() already does the registration you were going to do by hand: it builds PyModule_GetName(parent) + ".query", calls PyImport_AddModule() on it, so _hal.query lands in sys.modules and import _hal.query resolves, and it assigns the result as the query attribute of the parent. So no PyModule_AddObject() of your own.

No PyType_Ready() either. That is for static types you define with the plain C API; pybind11 finalizes its own types inside py::class_, and halquery has no classes, only free functions.

Then in halmodule.cc, in place of my placeholder block:

if(halquery_add_submodule(m) < 0) {
    Py_DECREF(m);
    return NULL;
}

Two ordering points:

• Call it after type, dir and comptype have been added to m. That is also why the enums are read from parent above rather than with py::module_::import("_hal"): during PyInit_hal the module is not in sys.modules yet, so that import would re-enter the import machinery.
• The GIL is held throughout module init, which is all pybind11 needs, and hal_lib_init()/hal_lib_exit() stay exactly as they are now that everything lives in one module.

Build side: fold hal/halquery.cc into HALMODULESRCS and drop the HALQUERY target and its PYTARGETS entry. One .so instead of two also removes the question of two pybind11 modules sharing internals across separate objects.

Unrelated to the mechanics, but it comes along for the ride: the three file-scope static py::object globals are destroyed at library unload, after Py_Finalize(), which is a Py_DECREF without an interpreter. Either release them from an atexit-style hook or make them intentionally leaked (static py::object *).

The fake module in my tree was only ever a placeholder for exactly this. Once the real bindings live in _hal, the PyModule_New() block goes away and the only piece that stays is the sys.modules["hal.query"] alias in hal.py, because hal is a plain module and not a package.

[BsAtHome]

PYBIND11_MODULE(halquery, m) is only a macro: it emits PyInit_halquery plus a function body that takes a py::module_ &. Nothing else in pybind11 depends on that entry point, so you can write the function yourself and hand it the _hal module object.

That was what I was missing. Thanks!

I had the idea that pybind11 would have had a construct for this as it probably isn't an impossible situation.

Two ordering points: [snip]

yes... all fixed now...

Unrelated to the mechanics, but it comes along for the ride: the three file-scope static py::object globals are destroyed at library unload, after Py_Finalize(), which is a Py_DECREF without an interpreter. Either release them from an atexit-style hook or make them intentionally leaked (static py::object *).

Yes, I thought about that they would leak. If they are not decref'ed, then they hold the enums and therefore the _hal module. But, it does not prevent Py_AtExit to call the hal_lib_exit() function, which is the real issue. The only time halmodule is unloaded is at program exit (or crash). Not sure if it is essential to do a full deconstruction. Does it matter?

[grandixximo]

No, it does not matter, and my remark was too strong: I rebuilt the pattern outside LinuxCNC and it is harmless.

What I am less sure of is whether we want to depend on why it is harmless. The static destructor does run after finalization, but the object still carries 10 references, so the decref never reaches tp_dealloc. That count is a CPython artifact: an IntEnum class is self-referential through its members and _value2member_map_, and the C-held reference sits outside the cycle, so the collector cannot take it either. It holds with the parent module on multi-phase init and dropped from sys.modules as well; the only shape I could get to abort was a plain acyclic object whose sole owner is the static.

That shape stops aborting when the global is a raw PyObject *, incref'd once at init and never released: same object, same sole ownership at refcount 1, exit 0 instead of Fatal Python error: PyThreadState_Get. It is py::object(...).release().ptr() at init, and the call sites take a py::handle(...) wrap; tagging still returns <HalType.BOOL: 1> as before.

So, a question I cannot answer for you: is "the refcount is never 1" something you want in the trust base, or would you rather those three globals never run a destructor at all? The pointer version is the same leak you have already accepted, without the dependency. Is there something that argues for keeping them as py::object?

[BsAtHome]

I'd hate to use raw PyObject pointers. Afaik, using py::object should have increased the refcount, so when the destructor (tries to) kills it, the underlying object should remain. Alternatively, we can enforce it by doing:

    halenumtype = p.attr("type");
    halenumdir  = p.attr("dir");
    halenumcomp = p.attr("comptype");
    halenumtype.inc_ref();
    halenumdir.inc_ref();
    halenumcomp.inc_ref();

Then we don't care about the C++ destructor and purposefully leak the reference.

[grandixximo]

Agreed, and better than my version: no call site changes and the C++ type stays. I ran it against the shape that aborts, a plain acyclic object whose only owner is the static, and with the inc_ref() it goes from Fatal Python error: PyThreadState_Get to refcount 2 at init, 1 after the destructor, exit 0, clean under PYTHONMALLOC=debug.

One thing to be careful about in the reasoning, though. The reference py::object takes is exactly the one its destructor gives back, so on its own it nets to zero and protects nothing; the object survives today only because something else still holds it. Your inc_ref() works because it adds a second reference, not because the first one is there.

Which is why I would put a line of comment next to the three calls. They read as redundant, and the next person tidying up will delete them.