Release v4.1.10 - #166
Merged
Merged
Conversation
Lets a vendor plugin that owns a physical RUN/STOP switch hold the PLC in stop, so hardware is authoritative over the editor. The transition flow is unchanged: request_plc_start mirrors the existing request_plc_stop, and both are thin wrappers over the same plc_begin_transition() the socket START/STOP handlers already drive. The only genuinely new state is one atomic holding the switch position. A plugin reports the position with set_switch_position() and then asks for the matching transition. Store first, then request: on a falling edge that closes the window where a start could slip in, and on a rising edge it stops the request being refused by the guard still reading a stale STOP. Sample the switch from a thread started in init() and torn down in cleanup(), not start_loop()/stop_loop() -- the runtime calls stop_loop when the PLC stops, which is exactly when the position matters most. Every start path consults the same guard: the socket START (now answering START:ERROR_SWITCH_STOP), the boot auto-start, and request_plc_start itself. Refused, never queued, so the editor can explain why. transition_worker gains a converge-on-switch re-check, because plc_begin_transition() returns false while another transition is in flight and a stop request can therefore be dropped -- which would leave the PLC running with the switch in STOP and nobody retrying. Re-checking after the gate clears makes every interleaving converge on STOPPED. This also fixes the same pre-existing drop for a plugin reporting a hardware fault during a start. Also fixes a pre-existing ABI bug in the Python ctypes mirror: it omitted request_plc_stop while the C struct had it, so every field after it was shifted one pointer and base_tick_ns was reading the request_plc_stop function pointer. Offsets and sizeof now match the C struct exactly (verified in a container: 552 bytes, all offsets equal). Verified on a Docker runtime with a test VPP plugin: boot gated with no start-then-stop bounce, START refused while STOP, and both switch edges driving transitions with no API call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix: Modbus/TCP: incorrect behavior when the connection is lost (issue #691)
Appending `request_plc_start`, `set_switch_position` and `get_plc_state` to
`plugin_runtime_args_t` kept BINARY compatibility, which is what the comment there
claims — and that is all it kept. A VPP plugin ships as SOURCE and is compiled on
the device against whatever runtime is installed, so a plugin referring to those
members fails to build on an older runtime:
synergy_mode.c:92: error: 'plugin_runtime_args_t' has no member named
'set_switch_position'
That is a confusing failure a long way from its cause, hit on a real SLM-RP4.
`PLUGIN_RUNTIME_ARGS_HAS_MODE_SWITCH` lets a plugin support both vintages with a
feature test, the source-level equivalent of the weak symbols the baremetal HAL
uses for the same purpose. Older runtimes simply do not define it, which is exactly
what makes the test work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
image_tables_bind_located_vars() derived the config-scope located set positionally: it assumed strucpp emitted program-local entries before CONFIGURATION VAR_GLOBAL ... AT entries, and took the tail not covered by any program's located_range() as the globals. strucpp emits config globals FIRST, so the program-local block is always the tail. covered_end therefore reached locatedVarsCount as soon as ANY POU declared a located variable, g_config_located_count collapsed to 0, and image_tables_copy_config_globals_in/out became no-ops -- every located configuration global silently stopped being synced. %MX coils written over Modbus never reached the program, and %IX/%QX/%MW globals stopped being read and committed. Reported on the forum as "%MX locations now invalid - Runtime v4": the reporter's %MX writes worked until he added one unrelated located variable to a POU, and downgrading to v4.1.3 (which predates the threaded process-image model) fixed it. The runtime cannot recover the scope on its own: globals are file-scope GlobalVar<V> singletons that appear nowhere in the configuration object graph, so ConfigurationInstance -> Resource -> Task -> Program never reaches them. STruC++ now states it instead, emitting locatedGlobals[] with the canonical storage pointer of every located VAR_GLOBAL -- the same raw_ptr() value it writes into locatedVars[].pointer. Replace the inferred slice with an explicit index list built once at load by joining locatedVars[].pointer against locatedGlobals[] on pointer identity. No ordering rule, no layout assumption, and no false positives since distinct objects have distinct addresses. The join lives in located_globals.c behind a pointer-accessor callback so it carries no strucpp layout knowledge and is unit-testable; tests/test_located_globals.c locks order-independence, the multi-program shape, the degradation paths, inconsistency detection and unbound descriptors. Backwards compatible in both directions. strucpp emits the extern "C" accessors beside its own array, so nothing here references a symbol an older program lacks -- important because scripts/compile.sh rebuilds the shim against each upload's generated header, where a shim-side reference would be a COMPILE error. The accessors are resolved with required=false (same pattern as strucpp_debug_locate): an older program resolves NULL, in which case located configuration globals are not synced at all and a warning naming the fix is logged once at load. The program still runs and POU-local located I/O is unaffected. An older runtime never looks the symbols up and is bit-for-bit unaffected. Two validations guard the result. Join totality: every locatedGlobals[] entry must match some located variable, otherwise the generated arrays disagree. And the program-range coverage bitmap is retained purely as an independent cross-check witness -- it can flag a config global that a task also claims, but it never decides a scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ature macro" This reverts commit fb4cec0. Compatibility between a VPP and the runtime it is compiled on belongs in version metadata — "this package requires runtime 4.1.9+" — not in per-feature macros a plugin has to test. Version gating is being added across the components anyway (`minEditorVersion` already exists in the manifest schema; the runtime half does not yet), and once it lands nothing would reference this macro. Better removed now than left as the kind of dead code that outlives the reason for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The program .so defined STB_GNU_UNIQUE symbols (strucpp's template statics, the debug type_ops table, statics in inline functions), and glibc will not unmap a DSO that defines them. dlclose() reported success, the runtime logged "PLC program unloaded successfully", and the mapping stayed resident with all its statics intact -- so the next start, which dlopens the same path, got the previous run's initialized image handed back by name match. Stop therefore stopped the scan threads but never reset the program: a CTU that had counted to 100 came back still at 100, and the program appeared to run while producing nothing. Both the editor's run/stop button and the SLM-RP4's mode switch hit it, because both go through plc_set_state(). Compiling the program with -fno-gnu-unique drops the attribute. Nothing else in the process defines these symbols (plc_main exports only the ext_strucpp_* hook pointers), so there is no cross-DSO identity to preserve. Verified on an SLM-RP4 through the normal upload path: the rebuilt .so has zero UNIQUE symbols, leaves /proc/<pid>/maps on every stop, and re-maps on every start. The previously built .so is still mapped in the same process and never comes back -- it was pinned for the process's lifetime.
fix(runtime): sync located globals from STruC++'s locatedGlobals[]
Rapid run/stop flips could wedge the runtime permanently: STATUS stuck at TRANSITIONING, every other command answering COMMAND:BUSY (which surfaced in the editor as "Could not verify MD5: Unexpected response format"), the editor showing stopped, and the program still scanning. Reproduced on hardware: 25 start/stop pairs 150 ms apart wedged at round 17. Root cause was two sources of truth. plc_state was written eagerly by whoever requested a change, and an is_transitioning atomic tracked overlap alongside it. plc_cycle_thread then re-asserted RUNNING unconditionally after being spawned, so a stop landing in the window before that thread was first scheduled wrote STOPPED, joined the thread, and had RUNNING put back underneath it. Every loop in that thread exits only on plc_get_state() != RUNNING, so it never exited, the join never returned, and the atomic was never cleared. The state is now the interlock. plc_claim_transition() publishes TRANSITIONING_TO_RUN or TRANSITIONING_TO_STOP under the state lock and refuses any request while one is in flight -- you cannot change state in the middle of changing state -- and plc_publish_final_state() ends it. Nothing writes a target state up front, so there is nothing left to resurrect, and there is no second flag to keep in step. RUNNING now means running: it is published immediately before the dispatcher releases the first scan, not when a start is requested. Both premature writes are gone, as is the mid-transition PLC_STATE_INIT write that made STATUS flicker to INIT and momentarily hid the fact that a transition was in flight. Audited every consumer that tests for RUNNING; none needed changing, and the watchdog's "reset the baseline when not running" behaviour makes a longer start safer rather than riskier. Dropped requests no longer lose the switch's intent. plc_switch records only that the switch MOVED, and the transition-completion path compares where it came to rest against the state actually reached, correcting a mismatch. Several flips during one transition collapse to the final position. Movement-gated on purpose: an editor stop with the switch untouched records nothing, so Stop still works with the switch in RUN. This is runtime-side, so every platform's VPP gets it without changes. The completion path waits to observe the landing before reconciling. A start returns as soon as the PLC thread is spawned and lands seconds later, so reconciling immediately discarded the movement while the state was still TRANSITIONING -- verified on hardware, then fixed. Watchdog backstop: TRANSITIONING persisting past a bound forces ERROR. With the state as the interlock there is no flag anyone could clear, so an unpublished transition would otherwise be an unrecoverable wedge. Verified on an SLM-RP4: transitions land in 0.6-0.8 s (~4 s for a boot start, almost all plugin bring-up); repeated start/stop pairs never stick; and a boot with the switch in STOP shows a plugin stop request dropped mid-start, the start landing RUNNING, then "Mode switch came to rest in STOP but the PLC landed on RUNNING — correcting" and convergence to STOPPED.
A plugin asking for a stop because someone moved the mode switch is normal operation, but it was logged at ERROR — so every ordinary flip to STOP painted the journal red, alongside the real errors. Now INFO, matching the start request it mirrors. Also corrects two comments that no longer describe the code. The dropped-request recovery is the switch-movement reconciliation, not the old converge-on-switch re-check. And plc_main's auto-start gate does NOT see a switch position reported by a VPP plugin, because that plugin initialises inside the start transition the gate is deciding about: a device powered up with the switch in STOP starts and is then corrected by reconciliation, rather than being held stopped up front. Worth saying plainly, since the old comment claimed the opposite.
Until now nothing let a runtime state what it needs from an editor: the upload endpoint accepts any ZIP that passes analyze_zip, and the bundle carries no editor identity at all. So "this runtime must not accept programs from an older editor" was not expressible. The new endpoint declares it. Unauthenticated, like /api/version — an editor calls it before login, because an editor too old to log in must still be able to find out why. The runtime only ADVERTISES this value; the editor compares it and refuses to upload. That is deliberate. Nothing on the upload path enforces it, so shipping a runtime release can never lock out an editor already installed in the field, and there is one place to debug when a push is refused. It is not a security control and should not be described as one. MIN_EDITOR_VERSION starts at 4.1.0 because that is where the STruC++ pipeline landed — 4.0.x editors emitted MatIEC artefacts this runtime cannot build at all. It is not a build counter: raising it for a release that merely changed something locks out working editors for no reason. Editors predating the endpoint get a 401 from the /<command> catch-all (not a 404) and fall back to /api/version, seeing no floor — exactly their previous behaviour. Verified against a real pre-change container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…compatibility feat(runtime): publish minEditorVersion at /api/capabilities (DOPE-448)
…e-bit-pattern fix(opcua): write REAL/LREAL values, not their bit pattern
Every finding here is one shape: the state said a transition was over while the work was still running. plc_state has 15 write sites across 3 threads and nothing distinguishes "I own this transition" from "I am writing a variable", so a landing published by a non-owner either erased a state someone else had landed or reported work that never happened. These are targeted fixes to the paths that do it; the single-owner refactor is tracked separately. Critical - plc_cycle_thread no longer publishes RUNNING unconditionally. New plc_publish_running_if_claimed() lands it only while the state is still TRANSITIONING_TO_RUN, doing the check and the write in one critical section; on refusal the thread reaps its workers and exits instead of entering the dispatcher loop. Paired with the guard below, since either one alone leaves the hang in place. - unload_plc_program publishes TRANSITIONING_TO_STOP for any non-ERROR state, not just RUNNING. On SIGTERM during a start the state is TRANSITIONING_TO_RUN, so the old guard published nothing, the cycle thread then published RUNNING underneath the teardown, and pthread_join(plc_thread) never returned -- systemd waited out its timeout and SIGKILLed a runtime that was still scanning. - plc_begin_transition completes the transition on the calling thread when the worker cannot be spawned, instead of publishing STOPPED. The claim has already published TRANSITIONING_TO_STOP, which is what makes the scan loops exit, so the old path ended the scan but skipped unload_plc_program entirely: journal_cleanup, plugin_driver_stop, plugin_manager_destroy and the dlclose were all skipped, plc_program stayed non-NULL, plc_thread was never joined, and STATUS reported a stop that tore nothing down. The next start then re-entered plugin_driver_init on live plugin state and re-ran a program whose statics were never reinitialised. Blocking the caller on an allocation or thread-exhaustion path is the cheaper cost. - plc_main stops the program before destroying the plugin driver. The teardown calls plugin_driver_stop(plugin_driver), so destroying the driver first left that reading freed memory on every shutdown with a program loaded. High - The stuck-transition bound is derived from the landing bound rather than being an independent number: PLC_TRANSITION_STUCK_TIMEOUT_MS is PLC_TRANSITION_LANDING_TIMEOUT_MS + 30 s, shared through plc_state_manager.h. The watchdog's 60 s used to fire 30 s BEFORE the runtime's own 90 s wait had given up, forcing ERROR while the transition worker was still executing -- which releases the interlock and lets a START be accepted from ERROR on top of a live load or unload. The window is now unreachable in normal operation; the comment on plc_force_error_state records that forcing ERROR still does not abort the work, which needs the transition owner to be able to abort itself. - Mode-switch reconciliation re-arms on refusal. plc_switch_take_movement() consumes the record before plc_begin_transition() is known to have started, and the return value was dropped -- so a request accepted in the gap between the landing and the correction lost the switch's intent entirely, with no retry and no log. New plc_switch_note_movement() hands the record back so the next landing reconciles. - SWITCH is answered during a transition. It is a plain atomic load of plc_switch with no coupling to plc_state, but COMMAND:BUSY made parse_switch_position return None, so the webserver dropped switchPosition from every status response for the whole duration of a start or stop and GET /switch answered "unknown" -- losing the field exactly while the editor polls through a transition it just requested. Medium - Worker SCHED_FIFO priority is clamped to 98, reserving 99 for the dispatcher. The dispatcher runs at 99 to be strictly above every worker, but the clamp allowed a task to TIE it, and SCHED_FIFO does not time-slice between equal priorities: an unbounded loop in IEC code at 99 keeps the dispatcher off that CPU entirely. - plc_set_state's header doc states its precondition (a claim must already have published a TRANSITIONING state) and points callers at plc_begin_transition. It still read like a setter, which is now a footgun: a direct call tears the program down with no claim and then publishes a final state nobody claimed. - Drop the dead `extern PLCState plc_state` in unix_socket.c; the definition is static, so nothing could ever have linked to it. Extract format_switch_response() and reap_task_threads() rather than duplicating either body. Not addressed here: a device that boots with the mode switch in STOP still scans for the duration of the start plus the corrective stop. Aborting a start safely needs one component owning both directions -- a veto inside plc_cycle_thread would leave the same partial teardown described above. The behaviour is documented at plc_main.c:171. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found while building an end-to-end test for the PR #162 findings: the runtime could not survive the very path those fixes are about. Four defects, all pre-existing on development, each verified by a test that fails without the fix. - SIGTERM had no handler. RuntimeManager stops the runtime with process.terminate() and systemd's default KillSignal is also SIGTERM, so the default disposition applied and every supervisor-initiated stop killed the process outright: the program was never unloaded, plugins never stopped, the journal never flushed, the program .so never dlclose'd. Installed alongside SIGINT; handle_sigint is now handle_shutdown_signal, since it serves both. - Py_FinalizeEx() ran without the GIL, segfaulting the process on every graceful shutdown of a runtime whose PLC had never started -- which includes safe mode, the crash-recovery path. plugin_driver_destroy releases the state PyGILState_Ensure() gave it and then restores main_tstate, but main_tstate was only ever assigned by plugin_driver_start(); plc_main called PyEval_SaveThread() directly and discarded the result. So a runtime that had run once exited cleanly and one that had not crashed in PyImport_GetModule. New plugin_driver_release_gil() keeps that bookkeeping next to the code that consumes it, and the destroy path no longer drops the GIL it needs. plugin_driver_start() no longer overwrites main_tstate either: it runs on the cycle thread, so a shutdown after a start was restoring the state of a thread that no longer existed. - The command socket accepted commands before the plugin driver was built. A START arriving in that window ran load_plc_program -> plugin_driver_init on the transition worker while the main thread was still inside plugin_driver_load_config/init, and rebuilding a slot dlcloses the .so -- so a plugin sleeping in its own init() returned into an unmapped page. Observed as a SIGSEGV in the main thread at an address inside the just-unloaded plugin. Transition arbitration cannot help: it is one transition racing driver setup, not two transitions. setup_unix_socket() now runs after the driver is ready. - plc_state_manager_cleanup() tore down on top of a transition in flight. During plugin bring-up load_plc_program has not assigned plc_thread yet, so the join ran on a handle that was never set; a moment later the cycle thread would have RUNNING published underneath the teardown. It now waits for the transition to land first, bounded by PLC_TRANSITION_LANDING_TIMEOUT_MS, which turns shutdown-during-a-start into an ordinary stop. RuntimeManager waits RUNTIME_SHUTDOWN_TIMEOUT_S (15 s) for the graceful stop before SIGKILL, since 5 s would preempt the cleanup SIGTERM now performs -- a boot start with plugin bring-up is ~4 s on an SLM-RP4. The kill remains the backstop for a teardown that hangs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Boots a real plc_main with a real compiled program and drives it over the command socket, judging it only by what an outside observer can see: socket replies, the journal, and /proc/<pid>/maps. The bugs in this area are not wrong return values but a state claiming a transition finished while the work is still running, so the mapping is what tells you whether a reported stop unloaded anything -- STATUS says STOPPED either way. Scores 24 checks. Against the branch before the review fixes, 9 fail; after, none do. Notably it catches the phantom stop directly: STOP:ERROR, the .so still mapped, no unload in the journal, with the injected spawn failure confirmed. - fakevpp_plugin.c stands in for a board's VPP package. Configurable sleeps in init()/stop_loop() widen a transition enough to aim a signal or a competing request at it (bring-up is ~150 ms in a container, and a stop tens of ms), and a file-driven mode switch drives set_switch_position + request_plc_start/stop exactly as the plugin contract prescribes. - failinject.c fails exactly one pthread_create, one-shot and self-disarming, so the "transition worker could not be spawned" path is reachable without inducing real thread exhaustion. - logserver.py accepts the log socket. Without it, --print-logs writes nothing to stdout either and every journal assertion silently passes. Two traps are documented in the README because both cost real time: keeping the Python plugins in plugins.conf is what initialises the interpreter, and without that the whole Py_FinalizeEx class is untestable while every test still passes; and a plugin must not start a thread in init(), because update_config dlcloses the .so on each start and the thread returns into an unmapped page. Still argued from code rather than observed, and listed as such: the re-arm branch of switch reconciliation (needs a refusal that cannot be forced without a hook), the watchdog forcing ERROR on a stuck transition (needs a 2-minute transition), and terminating a runaway IEC task (needs the abort ladder that does not exist). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…state feat(runtime): run/stop state control with a hardware mode switch
Local installs and the Windows installer payload read the repo-root VERSION file to answer /api/version and the discovery scan, so it has to be in sync with the release tag before the tag is pushed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release v4.1.10
Merges
developmentintomainfor the v4.1.10 runtime release.VERSIONhas been bumped tov4.1.10— this is the file localgit clone && ./install.shinstalls and the Windows installer payload read to answer
/api/versionand the discoveryscan, so it must match the tag that gets pushed after this merge.
What's in this release
Run/stop state control (PR #162)
transition interlock instead of a separate flag.
Editor compatibility (PR #163, DOPE-448)
GET /api/capabilitiesnow publishesminEditorVersion(floor4.1.0) alongsideruntimeVersionso the editor can gate uploads.Fixes
locatedGlobals[], and letdlcloseactuallyunload the program on stop (PR fix(runtime): sync located globals from STruC++'s locatedGlobals[] #161).
Release checklist
VERSIONbumped tov4.1.10v4.1.10onmainand push (triggers the Docker + Windows installer workflows)🤖 Generated with Claude Code