Repository navigation
hal: stop returns once no thread runs a function - #4694
Open
grandixximo wants to merge 1 commit into
Open
grandixximo wants to merge 1 commit into
grandixximo wants to merge 1 commit into
Conversation
hal_stop_threads() only cleared threads_running. A thread in the middle of its pass went on running the rest of its functions, so a halcmd stop followed by unloadrt, which is how linuxcnc shuts down, could unload a component while the thread was still executing its function, and rtapi_app crashed in thread_task(). Hannes Diethelm found it in tests whose functions keep the servo thread busy for most of the period (LinuxCNC#4669). Each thread now marks itself busy before it reads threads_running and clears the mark after its pass. In a user process hal_stop_threads() clears threads_running, then waits until no thread is busy, at most 1000 of the thread's periods; a stuck thread makes it fail with -ETIMEDOUT. With both flags sequentially consistent atomics, either the thread sees the stop or the stop sees the pass. Called from realtime code it does not wait, as before. delf with the threads stopped trusted threads_running alone. It now also waits until the thread has finished its pass, which matters for a stop from realtime code and for a stop that runs while delf waits. tests/hal-delf-live holds the thread inside a function across a stop; the stop must not return before the thread has left it.
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
1 participant
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.
Part of #4669. @hdiethelm, this is the
thread_tasksegfault you traced, with the acknowledgement kept per thread.hal_stop_threads()only clearedthreads_running. A thread in the middle of its pass went on calling the rest of its functions, sohalcmd stopfollowed byunloadrt, which is how linuxcnc shuts down, could unload a component while the thread was still running its code. #4630 madedelfandunloadrtwait while the threads run, but once they are stopped the entry is freed at once.busyflag before it readsthreads_runningand clears it after its pass. Both are sequentially consistent atomics, so either the pass sees the stop or the stop sees the pass.hal_stop_threads()clearsthreads_running, then waits until no thread is busy, at most 1000 of each thread's periods, and fails with-ETIMEDOUTafter that. Called from realtime code, as motmod's exit does, it does not wait.delfwith the threads stopped also waits for the thread to finish its pass. That covers a stop from realtime code and a stop that runs whiledelfwaits.HAL_VERgoes to 0x16 for the new field.tests/hal-delf-livenow holds the thread inside a function acrosshalcmd stop. On masterstopreturns with the thread still inside, 3 of 3 runs, and a followingunloadrtcrashes rtapi_app. With this PR the test passes. The full suite passes apart from blendmath, which does not build with GCC 14 (#4676).@BsAtHome, the wait is in ULAPI only, after your point on #4630 that realtime code should not call
rtapi_delay().