RunningYoung opened a new issue, #145:
URL: https://github.com/apache/dubbo-getty/issues/145
**What happened**:
When a Go provider using `dubbo-go` serves a Java Dubbo 2.6 consumer over
the Dubbo TCP/Hessian protocol, closing a session while reads, writes, and
packet statistics are still in flight triggers data races in
`github.com/apache/dubbo-getty v1.4.10`.
This was observed in a real Java 8 consumer/provider alignment test, not
only in a synthetic unit test. The consumer completed 78 Hessian requests (78
reads and 78 writes). The business assertions completed, but the same test
executed with the Go race detector reported two `dubbo-getty` races during
session shutdown. A separate `dubbo-go` registration cleanup panic
(`BaseRegistry.UnRegister(nil)`) was also observed in the test environment;
that is a different issue and is not used as evidence for this getty report.
Representative race locations include:
- `session.go`: `session.stop()` -> `Conn()` / `ReadTimeout()` /
`WriteTimeout()`
- `session.go`: `session.WritePkg()` and packet counter paths
- `session.go`: `session.gc()` assigning `s.Connection = nil`
- `connection.go`: TCP `Send()` / `recv()` accessing `t.conn`
- `connection.go`: TCP `CloseConn()` assigning `t.conn = nil`
The abbreviated race shape is:
```text
WARNING: DATA RACE
Read at ... github.com/apache/dubbo-getty.(*session).Conn / ReadTimeout /
WritePkg
Previous write at ... github.com/apache/dubbo-getty.(*session).gc
s.Connection = nil
WARNING: DATA RACE
Read at ... github.com/apache/dubbo-getty.(*gettyTCPConn).Send / recv
Previous write at ... github.com/apache/dubbo-getty.(*gettyTCPConn).CloseConn
t.conn = nil
```
**What you expected to happen**:
Concurrent TCP read, write, timeout/deadline updates, packet accounting,
session shutdown, and connection garbage collection should be race-free and
deterministic.
After `Session.Close`/`CloseConn` begins:
1. in-flight operations should either complete against a stable connection
snapshot or return a documented closed-connection error;
2. no goroutine should read or write `s.Connection`, `t.conn`, `reader`, or
`writer` concurrently without synchronization;
3. close should be idempotent and must not cause a panic, use-after-close,
truncated response, or reconnect loop;
4. the package's own tests should cover the shutdown interleavings under `go
test -race`.
**How to reproduce it (as minimally and precisely as possible)**:
Environment:
```text
OS: Linux amd64 (WSL2)
Go: go1.26.5
Java consumer: Java 8, Apache Dubbo 2.6
Go provider: dubbo-go v3.3.2-20260419
getty: github.com/apache/dubbo-getty v1.4.10
transport: Dubbo TCP + Hessian
```
Steps:
1. Start a Go provider using `dubbo-go` with the Dubbo TCP protocol and a
Hessian-compatible service.
2. Start a Java 8 / Dubbo 2.6 consumer against that provider.
3. Execute the normal request set (the observed test executes 78 Hessian
requests).
4. While the provider still has its read loop, asynchronous `WritePkg`
calls, and packet accounting callbacks active, close the consumer connection
and trigger provider session cleanup/stop.
5. Run the same integration test with the race detector enabled:
```bash
go test -race ./path/to/the/integration/test
```
6. Observe race reports in the getty session/connection shutdown path. The
exact number of reports can vary with scheduling, but the affected code paths
are the same.
The issue is easiest to reproduce when the peer disconnect and provider
shutdown/garbage collection overlap. It is a TCP issue; it is not dependent on
WebSocket transport.
**Anything else we need to know?**:
The current `v1.4.10` source has the following lifecycle pattern:
```go
// session.go
func (s *session) Conn() net.Conn {
if tc, ok := s.Connection.(*gettyTCPConn); ok {
return tc.conn
}
// ... other connection types
}
func (s *session) gc() {
s.lock.Lock()
conn = s.Connection
s.Connection = nil
s.lock.Unlock()
go conn.CloseConn(...)
}
// connection.go
func (t *gettyTCPConn) CloseConn(waitSec int) {
if t.conn != nil {
_ = t.conn.Close()
t.conn = nil
}
}
```
`gc()` protects its assignment with `s.lock`, but `Conn()` and parts of
`WritePkg()` access the same session connection state outside that lock. TCP
`Send()`/`recv()` dereference `t.conn` while `CloseConn()` closes and then
writes `t.conn = nil`; there is no connection-level snapshot/close
synchronization for this pointer. The packet lock does not protect every
deadline, connection-pointer, reader, or writer access.
I also compared the published versions after `v1.4.10`:
- `v1.4.11` changes reconnect limiting;
- `v1.4.12` adds a WebSocket write mutex;
- `v1.5.0` adds WebSocket read/write synchronization and related error
handling;
- the current `master` history (as of 2026-08-29) contains client
shutdown-deadlock and reconnect single-flight fixes.
In the downloaded source, these versions still retain the TCP
`session.Conn()` / `session.gc()` / `gettyTCPConn.CloseConn()` pointer
lifecycle shown above. Replacing `v1.4.10` with `v1.5.0` compiles the
provider's Dubbo package, but the real Java consumer `-race` scenario has not
passed and the TCP lifecycle code is unchanged. Downgrading to `v1.4.9` also
retains the same unsynchronized TCP pointer lifecycle and is therefore not a
fix.
Suggested implementation direction:
1. Make all session connection access use a lock-protected connection
snapshot, including `Conn`, `WritePkg`, timeout/deadline access, send, and
packet counters.
2. Add connection-level synchronization or an atomic close/snapshot state
for `gettyTCPConn.conn`; `Send` and `recv` should operate on a stable local
snapshot, while `CloseConn` should atomically detach and close exactly once.
3. Apply the same lifecycle rule to `reader`/`writer` initialization and
shutdown, including compressed connections.
4. Add a deterministic regression test that overlaps TCP read, write,
deadline update, `Session.Close`/`gc`, and reconnect, and run it repeatedly
with `go test -race`.
Acceptance criteria:
- zero race reports in repeated TCP close/disconnect tests;
- no panic or use-after-close;
- no response truncation for requests that completed before the close
boundary;
- idempotent close and bounded shutdown time;
- existing WebSocket behavior and reconnect tests remain green.
Please let me know if a standalone reproducer or full race stack trace is
needed. I can provide the integration harness and a sanitized full stack trace
separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
