Skip to content

asynManager: fail cleanly instead of hanging on an oversize traceIO truncate size#235

Merged
MarkRivers merged 1 commit into
epics-modules:masterfrom
physwkim:fix/trace-truncate-size-hang
Jul 17, 2026
Merged

asynManager: fail cleanly instead of hanging on an oversize traceIO truncate size#235
MarkRivers merged 1 commit into
epics-modules:masterfrom
physwkim:fix/trace-truncate-size-hang

Conversation

@physwkim

Copy link
Copy Markdown
Contributor

Problem

setTraceIOTruncateSize() takes a size_t, but its two callers pass a
signed value:

  • asynRecord's TSIZ field is epicsInt32 (asynRecord.c)
  • the iocsh command asynSetTraceIOTruncateSize(..., int size)
    (asynShellCommands.c)

A negative value such as -1 converts to a size_t near SIZE_MAX.
With that size the function called

epicsMutexMustLock(pasynBase->lockTrace);
if(size>ptracePvt->traceBufferSize) {
    free(ptracePvt->traceBuffer);
    ptracePvt->traceBuffer = callocMustSucceed(size, sizeof(char), ...);

callocMustSucceed() loops forever (epicsThreadSuspendSelf) when the
allocation can't be satisfied — and it does so while holding the global
pasynBase->lockTrace mutex, so every other thread that traces then blocks
on that mutex too. A single

caput <record>.TSIZ -1

wedges tracing for the whole IOC.

The pre-existing free() before the allocation is a second problem: even a
recoverable allocator would leave traceBuffer dangling on failure.

Fix

Allocate the replacement with calloc() and, on NULL, unlock and return
asynError — an unsatisfiable request fails cleanly instead of hanging.
Free the existing buffer only once the new one is in hand, so a failure
leaves the current trace buffer intact. Both entry points funnel through
this single owner, so the record and iocsh paths are fixed together.

Test

Adds an asynPortDriverTest case: setTraceIOTruncateSize((size_t)-1)
returns asynError, and a normal size still returns asynSuccess. Without
the fix the test hangs on the oversize request instead of completing
(241/241 with the fix).

…runcate size

setTraceIOTruncateSize() takes a size_t, but the two callers pass a
signed value: asynRecord's TSIZ field (epicsInt32) and the iocsh
asynSetTraceIOTruncateSize(..., int size).  A negative value such as -1
converts to a size_t near SIZE_MAX.  With that size the function called
callocMustSucceed(), which loops forever (epicsThreadSuspendSelf) when the
allocation cannot be satisfied -- and it does so while holding the global
pasynBase->lockTrace mutex, so every other thread that traces then blocks
too.  A single `caput <record>.TSIZ -1` wedges tracing for the whole IOC.

The old code also freed the existing traceBuffer before allocating the
replacement, so even a recoverable allocator would leave traceBuffer
dangling on failure.

Allocate the replacement with calloc() and, on NULL, unlock and return
asynError so an unsatisfiable request fails cleanly.  Free the existing
buffer only once the new one is in hand, so a failure leaves the current
trace buffer intact.  Both callers reach this single owner, so the fix
closes the record and iocsh entry points together.

Add an asynPortDriverTest case: setTraceIOTruncateSize((size_t)-1) returns
asynError and a normal size still succeeds.  Without the fix the test
hangs on the oversize request instead of completing.
@MarkRivers
MarkRivers merged commit 2521ba4 into epics-modules:master Jul 17, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants