nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

syslog: Don't allow blocking when in signal handler

Open pussuw opened this issue 1 year ago • 1 comments

Summary

Blocking while running a signal handler is not advisable, instead write the log string character by character.

There is also a potential for a deadlock, as discussed in #6618

Note: querying for rtcb->sigdeliver is not 100% ideal, as it only tells if a signal handler has been queued, not if it is running. However, it makes syslog safe / usable which is a debug feature anyhow.

Impact

Fixes syslog deadlock when CONFIG_SMP=y. Happens during ostest signal handler tests.

Testing

qemu-rv:ksmp64

pussuw avatar Oct 22 '24 13:10 pussuw

[Experimental Bot, please feedback here]

The PR summary is missing some key information:

  • What functional part of the code is being changed? Be specific, e.g., "the syslog implementation" or "signal handler execution".
  • How does the change exactly work? Briefly explain the technical approach, like "Instead of blocking on the mutex, the log message is written one character at a time."

Impact:

  • The impact section should clearly state YES/NO for each item, even if describing it further. For example:
    • Impact on user: YES (Users relying on syslog within signal handlers will no longer experience deadlocks, particularly in SMP configurations)
  • Impact on build: While likely NO, confirm this explicitly.
  • Impact on hardware: This seems relevant as the issue mentions qemu-rv:ksmp64. Clarify if specific architectures or boards are impacted.
  • Impact on documentation: Does this fix require any documentation updates?

Testing:

  • The provided testing information is very minimal.
    • Specify the build host details (OS, Compiler version).
    • Instead of just stating "qemu-rv:ksmp64", describe the specific test case used to reproduce the deadlock before and how the fix addresses it.

Overall:

The PR summary needs more detail, and the impact and testing sections require additional information for a comprehensive review.

nuttxpr avatar Oct 22 '24 13:10 nuttxpr