Alanxtl commented on code in PR #3701:
URL: https://github.com/apache/dubbo-go/pull/3701#discussion_r3849555374
##########
filter/accesslog/filter.go:
##########
@@ -90,21 +99,21 @@ type Filter struct {
ctx context.Context
cancel context.CancelFunc
shutdownOnce sync.Once
+ wg sync.WaitGroup // tracks the processLogs goroutine
}
func newFilter() filter.Filter {
- if accessLogFilter == nil {
- once.Do(func() {
- ctx, cancel := context.WithCancel(context.Background())
- accessLogFilter = &Filter{
- logChan: make(chan Data, LogMaxBuffer),
- fileCache: make(map[string]*os.File),
- ctx: ctx,
- cancel: cancel,
- }
- go accessLogFilter.processLogs()
- })
- }
+ once.Do(func() {
+ ctx, cancel := context.WithCancel(context.Background())
+ accessLogFilter = &Filter{
+ logChan: make(chan Data, LogMaxBuffer),
+ fileCache: make(map[string]*os.File),
+ ctx: ctx,
+ cancel: cancel,
+ }
+ accessLogFilter.wg.Add(1)
+ go accessLogFilter.processLogs()
+ })
return accessLogFilter
}
Review Comment:
Shutdown() 读取 accessLogFilter 时加了 filterMu，但 newFilter() 写入 accessLogFilter
时没有加同一个锁
最好先构造局部变量并完成 `wg.Add(1)`，再在锁内发布：
```go
f := &Filter{...}
f.wg.Add(1)
filterMu.Lock()
accessLogFilter = f
filterMu.Unlock()
go f.processLogs()
```
##########
filter/accesslog/filter.go:
##########
@@ -442,3 +436,16 @@ func (f *Filter) shutdown() {
}
})
}
+
+// waitProcessLogs waits for the processLogs goroutine to exit, returning
+// false if it does not stop within the timeout.
+func (f *Filter) waitProcessLogs(timeout time.Duration) bool {
+ done := make(chan struct{})
+ go func() { f.wg.Wait(); close(done) }()
+ select {
+ case <-done:
+ return true
+ case <-time.After(timeout):
+ return false
+ }
+}
Review Comment:
`waitProcessLogs()` 超时后，内部等待 `wg.Wait()` 的 goroutine
可能继续存在，这是强制关闭策略下可以接受的代价，测试最好覆盖这个场景。
--
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]
