(plc4x) branch develop updated: fix(plc4go): guard serial TransportInstance.Write against concurrent

[sr...@apache.org]

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 2b1095c45d fix(plc4go): guard serial TransportInstance.Write against concurrent
2b1095c45d is described below

commit 2b1095c45dbce86ff61cc7e7ab0763ff9204cec8
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Mon Oct 12 09:28:15 2026 +0200

    fix(plc4go): guard serial TransportInstance.Write against concurrent
    
    Close nils serialPort/armer/pacer under stateChangeMutex, but Write and
    SetReadDeadline read those fields unguarded: a data race, and a nil-pointer
    panic when Close lands between Write's nil check and its use of the port.
    
    Snapshot the fields under the lock (not held across the blocking write);
    a write racing Close now fails on the closed port instead.
    
    Closes #2802
---
 plc4go/spi/transports/serial/TransportInstance.go  | 20 ++++++++++----
 .../transports/serial/TransportInstance_test.go    | 32 ++++++++++++++++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/plc4go/spi/transports/serial/TransportInstance.go b/plc4go/spi/transports/serial/TransportInstance.go
index f99fc91284..c100035b1a 100644
--- a/plc4go/spi/transports/serial/TransportInstance.go
+++ b/plc4go/spi/transports/serial/TransportInstance.go
@@ -172,10 +172,16 @@ func (m *TransportInstance) IsConnected() bool {
 }
 
 func (m *TransportInstance) Write(ctx context.Context, data []byte) error {
-	if !m.connected.Load() {
+	// Snapshot the port and pacer under the state lock: Close nils both, and
+	// a concurrent Close must not race the write (it fails on the closed
+	// port instead). The lock is not held across the blocking write.
+	m.stateChangeMutex.Lock()
+	connected, serialPort, writePacer := m.connected.Load(), m.serialPort, m.pacer
+	m.stateChangeMutex.Unlock()
+	if !connected {
 		return errors.New("error writing to transport. Not connected")
 	}
-	if m.serialPort == nil {
+	if serialPort == nil {
 		return errors.New("error writing to transport. No writer available")
 	}
 	var deadline time.Time
@@ -184,14 +190,14 @@ func (m *TransportInstance) Write(ctx context.Context, data []byte) error {
 	} else if m.cfg.writeTimeout > 0 {
 		deadline = time.Now().Add(m.cfg.writeTimeout)
 	}
-	if err := m.pacer.waitTurn(deadline); err != nil {
+	if err := writePacer.waitTurn(deadline); err != nil {
 		return errors.Wrap(err, "error pacing write")
 	}
-	if err := m.serialPort.SetWriteDeadline(deadline); err != nil {
+	if err := serialPort.SetWriteDeadline(deadline); err != nil {
 		return errors.Wrap(err, "error setting write deadline")
 	}
-	num, err := m.serialPort.Write(data)
-	m.pacer.noteActivity()
+	num, err := serialPort.Write(data)
+	writePacer.noteActivity()
 	if err != nil {
 		return errors.Wrap(err, "error writing")
 	}
@@ -206,7 +212,9 @@ func (m *TransportInstance) GetReader() transports.ExtendedReader {
 }
 
 func (m *TransportInstance) SetReadDeadline(deadline time.Time) error {
+	m.stateChangeMutex.Lock()
 	armer := m.armer
+	m.stateChangeMutex.Unlock()
 	if armer == nil {
 		return errors.New("error setting read deadline. Not connected")
 	}
diff --git a/plc4go/spi/transports/serial/TransportInstance_test.go b/plc4go/spi/transports/serial/TransportInstance_test.go
index 0ecab2bdbe..740f35ea99 100644
--- a/plc4go/spi/transports/serial/TransportInstance_test.go
+++ b/plc4go/spi/transports/serial/TransportInstance_test.go
@@ -388,3 +388,35 @@ func TestWrite_DedicatedInterframePacing(t *testing.T) {
 	require.NoError(t, m.Write(context.Background(), []byte{0x02}))
 	assert.GreaterOrEqual(t, time.Since(start), 40*time.Millisecond, "second write must wait out the gap")
 }
+
+// Close nils serialPort/armer/pacer under stateChangeMutex; Write and
+// SetReadDeadline must not read those fields unguarded (GH-2802). Run under
+// -race: the detector flags the unguarded read, and without it a Close landing
+// between Write's nil check and its use of the port panics.
+func TestTransportInstance_WriteAndSetReadDeadlineRaceClose(t *testing.T) {
+	for range 50 {
+		m := NewTransportInstanceWithConfig("test", func() serialConfig {
+			c := defaultSerialConfig()
+			c.interframeDelay = time.Microsecond
+			return c
+		}(), NewTransport())
+		m.serialPort = stubPort{}
+		m.pacer = newPacer(m.cfg.interframeDelay)
+		m.armer = newDeadlineReader(m.serialPort, m.cfg.readTimeout, m.pacer)
+		m.connected.Store(true)
+
+		done := make(chan struct{})
+		go func() {
+			defer close(done)
+			for range 100 {
+				_ = m.Write(context.Background(), []byte{0x01})
+				_ = m.SetReadDeadline(time.Time{})
+			}
+		}()
+		require.NoError(t, m.Close())
+		<-done
+
+		assert.Error(t, m.Write(context.Background(), []byte{0x01}), "write after close must fail")
+		assert.Error(t, m.SetReadDeadline(time.Time{}), "set read deadline after close must fail")
+	}
+}