You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "222twotwotwo (via GitHub)" <gi...@apache.org> on 2026/07/23 18:20:47 UTC
[PR] Fix wss event loop panics on normal server shutdown/error return paths [dubbo-getty]
222twotwotwo opened a new pull request, #138:
URL: https://github.com/apache/dubbo-getty/pull/138
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
This PR handles expected HTTP server shutdown errors in the WS/WSS event loops.
For WSS servers, normal shutdown or listener close errors are now treated as expected close paths instead of causing a panic. Unexpected WSS `Serve` errors still keep the original behavior: they are logged and then panic.
It also adds a regression test to verify that closing a WSS server does not panic during normal shutdown.
**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #132
**Special notes for your reviewer**:
The WSS panic behavior is preserved for unexpected `Serve` failures. This PR only excludes expected shutdown errors such as `http.ErrServerClosed`, `net.ErrClosed`, and closed listener errors.
Validated with:
```bash
go test ./...
```
**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
WSS server shutdown no longer panics when the underlying HTTP server or listener is closed normally.
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix wss event loop panics on normal server shutdown/error return paths [dubbo-getty]
Posted by "222twotwotwo (via GitHub)" <gi...@apache.org>.
222twotwotwo commented on code in PR #138:
URL: https://github.com/apache/dubbo-getty/pull/138#discussion_r3642738151
##########
server.go:
##########
@@ -421,11 +422,29 @@ func (s *server) runWSEventLoop(newSession NewSessionCallback) {
s.lock.Unlock()
err = server.Serve(s.streamListener)
if err != nil {
- log.Errorf("http.server.Serve(addr{%s}) = err:%+v", s.addr, perrors.WithStack(err))
+ if isHTTPServeCloseError(err) {
+ s.logHTTPServeClose(err)
+ } else {
+ s.logHTTPServeError(err)
+ }
}
}()
}
+func (s *server) logHTTPServeClose(err error) {
+ log.Infof("http.server.Serve(addr{%s}) closed: %v", s.addr, err)
+}
+
+func isHTTPServeCloseError(err error) bool {
+ return errors.Is(err, http.ErrServerClosed) ||
+ errors.Is(err, net.ErrClosed) ||
+ strings.Contains(err.Error(), "use of closed network connection")
Review Comment:
收到，已去除该文本匹配
##########
server_test.go:
##########
@@ -226,6 +226,30 @@ func TestWSServeWSRequestClosesSelfConnectConn(t *testing.T) {
}
}
+func TestWSSServerCloseDoesNotPanic(t *testing.T) {
+ certFile := filepath.Join(t.TempDir(), "server.crt")
+ keyFile := filepath.Join(t.TempDir(), "server.key")
Review Comment:
收到，已补充
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix wss event loop panics on normal server shutdown/error return paths [dubbo-getty]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #138:
URL: https://github.com/apache/dubbo-getty/pull/138
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix wss event loop panics on normal server shutdown/error return paths [dubbo-getty]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on PR #138:
URL: https://github.com/apache/dubbo-getty/pull/138#issuecomment-5128075926
@222twotwotwo good work. I have merged this PR. please continue your work if there are other problems.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix wss event loop panics on normal server shutdown/error return paths [dubbo-getty]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #138:
URL: https://github.com/apache/dubbo-getty/pull/138#discussion_r3642579550
##########
server_test.go:
##########
@@ -226,6 +226,30 @@ func TestWSServeWSRequestClosesSelfConnectConn(t *testing.T) {
}
}
+func TestWSSServerCloseDoesNotPanic(t *testing.T) {
+ certFile := filepath.Join(t.TempDir(), "server.crt")
+ keyFile := filepath.Join(t.TempDir(), "server.key")
Review Comment:
[P1] 补一条 unexpected Serve error 仍会失败的回归测试
当前用例只证明正常 `Close()` 不再触发 panic，没有保护本 PR 同时承诺保留的 fatal-error 路径。我在独立副本中把 `isHTTPServeCloseError` 变异为无条件返回 `true` 后，这个测试重复 20 次和全仓 `go test ./...` 仍全部通过，也就是所有 WSS Serve 故障被静默降级的回归无法被发现。请用可注入的 fake listener，必要时放到子进程中，分别验证 `http.ErrServerClosed`/wrapped `net.ErrClosed` 正常退出，而普通 fatal Accept/Serve error 必须进入 error 日志并保持 panic。
##########
server.go:
##########
@@ -421,11 +422,29 @@ func (s *server) runWSEventLoop(newSession NewSessionCallback) {
s.lock.Unlock()
err = server.Serve(s.streamListener)
if err != nil {
- log.Errorf("http.server.Serve(addr{%s}) = err:%+v", s.addr, perrors.WithStack(err))
+ if isHTTPServeCloseError(err) {
+ s.logHTTPServeClose(err)
+ } else {
+ s.logHTTPServeError(err)
+ }
}
}()
}
+func (s *server) logHTTPServeClose(err error) {
+ log.Infof("http.server.Serve(addr{%s}) closed: %v", s.addr, err)
+}
+
+func isHTTPServeCloseError(err error) bool {
+ return errors.Is(err, http.ErrServerClosed) ||
+ errors.Is(err, net.ErrClosed) ||
+ strings.Contains(err.Error(), "use of closed network connection")
Review Comment:
[P2] 不要仅按错误文本判定正常关闭
这里的 substring 会把不属于关闭的真实故障一起吞掉。我用真实 `http.Server.Serve` 和故障 listener 返回 `fatal listener failure after use of closed network connection marker`，当前 helper 会判为正常关闭，WSS loop 因而跳过 `logHTTPServeError` 和 panic。仓库最低 Go 版本已能通过 `errors.Is(err, net.ErrClosed)` 识别标准 listener 关闭；建议移除文本匹配，或者至少与明确的 shutdown 状态/sentinel 链联合约束，并补对应分类测试。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org