Skip to content

Conversation

@anjiahao1
Copy link
Contributor

Summary

  • Why change is necessary: Mutex recursion (a task attempting to lock a mutex it already holds) is not allowed in NuttX and indicates a programming error. Currently, such attempts may fail silently or cause unexpected behavior. Adding a debug assertion helps catch these bugs early during development and testing.

  • What functional part of the code is being changed: The nxsem_wait_slow() function in sched/semaphore/sem_wait.c, which handles the slow path of semaphore/mutex acquisition.

  • How does the change exactly work: A DEBUGASSERT is added to verify that the current task (obtained via nxsched_gettid()) is not already the holder of the mutex being acquired. The assertion checks the mutex holder bits (excluding the blocking bit) against the current task ID. If a task attempts to recursively lock a mutex it already holds, the assertion will trigger, alerting developers to the bug.

  • Related NuttX Issue reference: None

  • Related NuttX Apps Issue / Pull Request reference: None

Impact

  • Is new feature added? Is existing feature changed? YES - Existing feature enhanced. A debug-time safety check is added to the mutex acquisition path to detect illegal recursion attempts.

  • Impact on user: NO - This is a debug-only assertion (DEBUGASSERT) that only triggers in debug builds. Production builds are unaffected. Users who have mutex recursion bugs will now get clear assertion failures instead of silent failures.

  • Impact on build: NO - The change is minimal (5 lines added) and does not affect build process or configuration.

  • Impact on hardware: NO - This is a pure software change in the scheduler/semaphore subsystem with no hardware-specific implications.

  • Impact on documentation: NO - No documentation updates required. This is an internal debug enhancement.

  • Impact on security: NO - This change improves robustness by catching programming errors earlier, which is a positive security practice.

  • Impact on compatibility: NO - Backward compatible. Debug assertions are non-breaking; they only help catch bugs.

  • Anything else to consider: This change follows NuttX best practices for defensive programming. The assertion is placed at the critical point where mutex recursion would be detected, making it effective for catching bugs during development and testing.

Testing

I confirm that changes are verified on local setup and works as intended:

  • Build Host(s): ubuntu24.04 + arm-none-eabi-gcc 12.2.1
  • Target(s): mps3-an547:nsh
  • Run command: qemu-system-arm -M mps3-an547 -m 2G -nographic -kernel nuttx.bin

Testing logs before change:

N/A (baseline log not captured - this is a debug assertion enhancement)

Testing logs after change:

qemu-system-arm -M mps3-an547 -m 2G -nographic -kernel nuttx.bin

NuttShell (NSH) NuttX-12.7.2-vela
nsh>
nsh> ostest
stdio_test: write fd=1
stdio_test: Standard I/O Check: printf
stdio_test: write fd=2
ostest_main: putenv(Variable1=BadValue3)
ostest_main: setenv(Variable1, GoodValue1, TRUE)
ostest_main: setenv(Variable2, BadValue1, FALSE)
ostest_main: setenv(Variable2, GoodValue2, TRUE)
ostest_main: setenv(Variable3, GoodValue3, FALSE)
ostest_main: setenv(Variable3, BadValue2, FALSE)
show_variable: Variable=Variable1 has value=GoodValue1
show_variable: Variable=Variable2 has value=GoodValue2
show_variable: Variable=Variable3 has value=GoodValue3
ostest_main: Started user_main at PID=4

user_main: Begin argument test
user_main: Started with argc=5
user_main: argv[0]="ostest"
user_main: argv[1]="Arg1"
user_main: argv[2]="Arg2"
user_main: argv[3]="Arg3"
user_main: argv[4]="Arg4"

End of test memory usage:
VARIABLE  BEFORE   AFTER
======== ======== ========
arena    801fb944 801fb944
ordblks         2        2
mxordblk 7ffffff0 7ffffff0
uordblks     641c     641c
fordblks 801f5528 801f5528

user_main: getopt() test
getopt():  Simple test
getopt():  Invalid argument
getopt():  Missing optional argument
getopt_long():  Simple test
getopt_long():  No short options
getopt_long():  Argument for --option=argument
getopt_long():  Invalid long option
getopt_long():  Mixed long and short options
getopt_long():  Invalid short option
getopt_long():  Missing optional arguments
getopt_long_only():  Mixed long and short options
getopt_long_only():  Single hyphen long options

[... extensive test output showing all tests passing ...]

user_main: mutex test
Initializing mutex
Starting thread 1
Starting thread 2
                Thread1 Thread2
        Loops   32      32
        Errors  0       0

Testing moved mutex
Starting moved mutex thread 1
Starting moved mutex thread 2
                Thread1 Thread2
        Moved Loops     32      32
        Moved Errors    0       0

[... all remaining tests pass successfully ...]

user_main: vfork() test
vfork_test: Child 85 ran successfully

Final memory usage:
VARIABLE  BEFORE   AFTER
======== ======== ========
arena    801fb944 801fb944
ordblks         2        6
mxordblk 7ffffff0 7ffffff0
uordblks     641c     64b4
fordblks 801f5528 801f5490
user_main: Exiting
ostest_main: Exiting with status 0
stdio_test: Standard I/O Check: fprintf to stderr
nsh> QEMU: Terminated

Test Result: PASS - All ostest tests completed successfully, including mutex tests. The debug assertion does not interfere with normal mutex operation. No assertion failures detected, indicating no mutex recursion attempts in the test suite.

PR verification Self-Check

  • This PR introduces only one functional change.
  • I have updated all required description fields above.
  • My PR adheres to Contributing Guidelines and Documentation (git commit title and message, coding standard, etc).
  • My PR is still work in progress (not ready for review).
  • My PR is ready for review and can be safely merged into a codebase.

Add DEBUGASSERT in nxsem_wait_slow() to catch illegal mutex
recursion attempts. This helps identify bugs where a task tries
to lock a mutex it already holds, which is not allowed.

Signed-off-by: anjiahao <anjiahao@xiaomi.com>
@github-actions github-actions bot added Area: OS Components OS Components issues Size: XS The size of the change in this PR is very small labels Feb 3, 2026
@linguini1 linguini1 merged commit af0eac7 into apache:master Feb 3, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OS Components OS Components issues Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants