man-mu commented on code in PR #3701:
URL: https://github.com/apache/dubbo-go/pull/3701#discussion_r3853016258


##########
filter/accesslog/filter.go:
##########
@@ -431,6 +437,13 @@ func (f *Filter) shutdown() {
                        close(f.logChan)
                }
 
+               // Wait for processLogs to exit (drainLogs timeout is 5s, use 
6s margin)
+               select {
+               case <-f.done:
+               case <-time.After(shutdownWaitTimeout):
+                       logger.Warn("[Filter][AccessLog] shutdown wait for 
processLogs timeout")
+               }
+
                // Close all cached file handles

Review Comment:
   两个问题都已修复(77c0854d):
   
   **P1-1 初始化竞态**:按建议改为先构造局部变量并完成 `wg.Add(1)`,再在 `filterMu` 内发布,锁外启动 
goroutine。发布后的裸读安全性分三条路径:同 goroutine 内靠程序序;其他 `newFilter()` 调用方靠 `sync.Once` 的 
happens-before;`Shutdown()` 靠同一把 `filterMu` 配对,三者互不依赖。
   
   **P1-2 超时路径提前关文件**:超时后不再立即关闭句柄,改为后台 goroutine 等 `wg.Wait()` 完成后再 
`closeFiles()`,也就是不关闭仍由 writer 持有的文件。排查中发现日志文件以 `O_RDWR` 打开,FIFO 
场景下进程自持读端,阻塞中的写无法被 EPIPE 唤醒,因此我认为"等退出再关"是超时路径下相对安全的收尾方式。`drainLogs` 的 deadline 
确实无法中断单次阻塞写,但文件关闭时机已推迟到 writer 实际退出之后,deadline 现在只影响是否继续 drain,故维持现状。
   
   新增两个回归测试:
   
   - `TestAccessLogFilterConcurrentInitAndShutdown`:首次初始化与 Shutdown 从同一 barrier 
并发启动,循环 100 轮;
   - `TestAccessLogFilterShutdownTimeoutBlockedWriter`(unix only):用无消费端的 FIFO 让 
`WriteString` 阻塞触发超时路径,验证句柄未被提前关闭、writer 解除阻塞后由后台收尾关闭。
   
   `go test -race -count=30 ./filter/accesslog/` 通过。



-- 
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]

Reply via email to