This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
The following commit(s) were added to refs/heads/develop by this push:
new e23e01b44c fix(plc4go): prevent codec Disconnect deadlock on
transport-error re-entry
e23e01b44c is described below
commit e23e01b44c3d56b08c5582fb5d0075e57a95f144
Author: Sebastian Rühl <[email protected]>
AuthorDate: Mon Jun 29 15:09:15 2026 +0200
fix(plc4go): prevent codec Disconnect deadlock on transport-error re-entry
The transport-error handler registered by DefaultConnection invalidates and
closes the connection, which re-enters defaultCodec.Disconnect(). Because
emitTransportError was invoked inline from the receive worker, Disconnect()
would block on activeWorker.Wait() for the very worker stuck inside the
handler, while a concurrent Disconnect() from a connection-cache lease
return
held stateChange and the re-entrant call blocked acquiring it - a permanent
deadlock that wedged the codec (observed in the field: a Modbus poll stuck
>11h, dispatcher and discovery worker frozen, ~2700 goroutines).
Dispatch the handler on a tracked goroutine (m.wg) so the receive worker can
return and exit, and add a lock-free running==false fast path to
Disconnect()
so the re-entrant call returns before contending on stateChange/m.wg.
---
plc4go/spi/default/DefaultCodec.go | 34 +++++++++++++++++++++++++++++++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/plc4go/spi/default/DefaultCodec.go
b/plc4go/spi/default/DefaultCodec.go
index aa61c14fc8..9a6c40b487 100644
--- a/plc4go/spi/default/DefaultCodec.go
+++ b/plc4go/spi/default/DefaultCodec.go
@@ -179,6 +179,15 @@ func (m *defaultCodec) Connect(ctx context.Context) error {
}
func (m *defaultCodec) Disconnect() error {
+ // Lock-free fast path. A transport-error handler dispatched via
+ // emitTransportError runs on m.wg and may call back into Disconnect
(through
+ // connection.Invalidate -> Close) while another Disconnect already
holds
+ // stateChange and is blocked in m.wg.Wait(). The error paths store
+ // running=false before emitting, so this check lets the re-entrant call
+ // return before it contends on stateChange, breaking that deadlock
cycle.
+ if !m.running.Load() {
+ return errors.New("already disconnected")
+ }
m.stateChange.Lock()
defer m.stateChange.Unlock()
if !m.running.Load() {
@@ -602,9 +611,29 @@ func (m *defaultCodec) handleTransportError(workerLog
zerolog.Logger, err error)
}
func (m *defaultCodec) emitTransportError(kind transports.TransportErrorKind,
err error) {
- if m.transportErrorHandler != nil {
- m.transportErrorHandler(kind, err)
+ handler := m.transportErrorHandler
+ if handler == nil {
+ return
}
+ // The handler is external code (typically the owning connection) that
may
+ // react to a fatal error by invalidating and closing the connection,
which
+ // calls back into Disconnect(). Disconnect() blocks on
activeWorker.Wait()
+ // until the receive/expire workers have exited and contends on
stateChange.
+ // emitTransportError is invoked *from* the receive worker, so calling
the
+ // handler inline would make that worker wait for itself - or deadlock
+ // against a concurrent Disconnect() that already holds stateChange and
is
+ // waiting for this worker to exit. Dispatch it on a goroutine so the
worker
+ // can return and exit. The re-entrant Disconnect() is safe here
because its
+ // lock-free fast path returns before contending on stateChange/m.wg
once
+ // shutdown has been signalled (running == false).
+ m.wg.Go(func() {
+ defer func() {
+ if r := recover(); r != nil {
+ m.log.Error().Interface("panic",
r).Msg("recovered from panic in transport error handler")
+ }
+ }()
+ handler(kind, err)
+ })
}
func (m *defaultCodec) failAllExpectations(err error) {
@@ -614,7 +643,6 @@ func (m *defaultCodec) failAllExpectations(err error) {
m.expectationsChangeMutex.Unlock()
for _, expectation := range expectations {
- expectation := expectation
expectation.Cancel(err)
if handleErr := expectation.GetHandleError(); handleErr != nil {
m.wg.Go(func() {