This is an automated email from the ASF dual-hosted git repository.
Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new 46026183c fix(filter, registry): close access log files on error paths
(#3561)
46026183c is described below
commit 46026183c06a0ce4576ff83e89eb80211270322b
Author: Nene7ko_ <[email protected]>
AuthorDate: Wed Aug 5 07:45:06 2026 +0800
fix(filter, registry): close access log files on error paths (#3561)
* fix: close access log files on error paths
* add test
---
filter/accesslog/file_handle_test.go | 138 +++++++++++++++++++++
filter/accesslog/filter.go | 7 ++
registry/servicediscovery/store/cache_manager.go | 5 +-
.../store/cache_manager_fd_test.go | 65 ++++++++++
4 files changed, 213 insertions(+), 2 deletions(-)
diff --git a/filter/accesslog/file_handle_test.go
b/filter/accesslog/file_handle_test.go
new file mode 100644
index 000000000..9b66be88e
--- /dev/null
+++ b/filter/accesslog/file_handle_test.go
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package accesslog
+
+import (
+ "os"
+ "path/filepath"
+ "runtime"
+ "testing"
+ "time"
+)
+
+func TestOpenLogFileClosesRotatedHandle(t *testing.T) {
+ if runtime.GOOS != "linux" {
+ t.Skip("requires Linux /proc fd accounting")
+ }
+
+ accessLog := filepath.Join(t.TempDir(), "access.log")
+ if err := os.WriteFile(accessLog, []byte("old log"), LogFileMode); err
!= nil {
+ t.Fatal(err)
+ }
+ filter := &Filter{}
+
+ before := countOpenFiles(t)
+ for range 128 {
+ staleTime := time.Now().Add(-48 * time.Hour)
+ if err := os.Chtimes(accessLog, staleTime, staleTime); err !=
nil {
+ t.Fatal(err)
+ }
+ logFile, err := filter.openLogFile(accessLog)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := logFile.Close(); err != nil {
+ t.Fatal(err)
+ }
+ }
+ after := countOpenFiles(t)
+ if after > before+2 {
+ t.Fatalf("open file descriptors grew from %d to %d", before,
after)
+ }
+}
+
+func TestOpenLogFileClosesHandleOnRenameError(t *testing.T) {
+ if runtime.GOOS != "linux" {
+ t.Skip("requires Linux file permission behavior")
+ }
+
+ dir := t.TempDir()
+ accessLog := filepath.Join(dir, "access.log")
+ if err := os.WriteFile(accessLog, []byte("old log"), LogFileMode); err
!= nil {
+ t.Fatal(err)
+ }
+ if err := os.Chmod(dir, 0o500); err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { _ = os.Chmod(dir, 0o700) })
+
+ filter := &Filter{}
+ before := countOpenFiles(t)
+ for range 128 {
+ staleTime := time.Now().Add(-48 * time.Hour)
+ if err := os.Chtimes(accessLog, staleTime, staleTime); err !=
nil {
+ t.Fatal(err)
+ }
+ logFile, err := filter.openLogFile(accessLog)
+ if err == nil {
+ _ = logFile.Close()
+ t.Skip("environment permits rename in a read-only
directory")
+ }
+ }
+ after := countOpenFiles(t)
+ if after > before+2 {
+ t.Fatalf("open file descriptors grew from %d to %d", before,
after)
+ }
+}
+
+func TestGetOrOpenLogFileClosesCachedFileOnOpenError(t *testing.T) {
+ if runtime.GOOS != "linux" {
+ t.Skip("requires Linux file descriptor behavior")
+ }
+
+ dir := t.TempDir()
+ accessLog := filepath.Join(dir, "access.log")
+ oldFile, err := os.OpenFile(accessLog,
os.O_CREATE|os.O_APPEND|os.O_RDWR, LogFileMode)
+ if err != nil {
+ t.Fatal(err)
+ }
+ staleTime := time.Now().Add(-48 * time.Hour)
+ err = os.Chtimes(accessLog, staleTime, staleTime)
+ if err != nil {
+ _ = oldFile.Close()
+ t.Fatal(err)
+ }
+ err = os.Remove(accessLog)
+ if err != nil {
+ _ = oldFile.Close()
+ t.Fatal(err)
+ }
+ err = os.Mkdir(accessLog, 0o700)
+ if err != nil {
+ _ = oldFile.Close()
+ t.Fatal(err)
+ }
+
+ filter := &Filter{fileCache: map[string]*os.File{accessLog: oldFile}}
+ _, err = filter.getOrOpenLogFile(accessLog)
+ if err == nil {
+ t.Fatal("getOrOpenLogFile should return an error when OpenFile
targets a directory")
+ }
+ if closeErr := oldFile.Close(); closeErr == nil {
+ t.Fatal("cached file was not closed before OpenFile failed")
+ }
+}
+
+func countOpenFiles(t *testing.T) int {
+ t.Helper()
+ entries, err := os.ReadDir("/proc/self/fd")
+ if err != nil {
+ t.Fatal(err)
+ }
+ return len(entries)
+}
diff --git a/filter/accesslog/filter.go b/filter/accesslog/filter.go
index 02a1f8e4f..8330ba4b2 100644
--- a/filter/accesslog/filter.go
+++ b/filter/accesslog/filter.go
@@ -328,6 +328,9 @@ func (f *Filter) openLogFile(accessLog string) (*os.File,
error) {
fileInfo, err := logFile.Stat()
if err != nil {
logger.Warnf("[Filter][AccessLog] can not get the info of
access log file, accessLog=%s err=%v", accessLog, err)
+ if closeErr := logFile.Close(); closeErr != nil {
+ logger.Warnf("[Filter][AccessLog] failed to close
access log file, accessLog=%s err=%v", accessLog, closeErr)
+ }
return nil, err
}
last := fileInfo.ModTime().Format(FileDateFormat)
@@ -339,6 +342,10 @@ func (f *Filter) openLogFile(accessLog string) (*os.File,
error) {
// By this way, we can split the access log based on days.
// use 'accessLog' as complete path to avoid log not found.
if now != last {
+ if closeErr := logFile.Close(); closeErr != nil {
+ logger.Warnf("[Filter][AccessLog] failed to close
access log file before rotation, accessLog=%s err=%v", accessLog, closeErr)
+ return nil, closeErr
+ }
err = os.Rename(accessLog, accessLog+"."+now)
if err != nil {
logger.Warnf("[Filter][AccessLog] can not rename access
log file, accessLog=%s err=%v", accessLog, err)
diff --git a/registry/servicediscovery/store/cache_manager.go
b/registry/servicediscovery/store/cache_manager.go
index ad3b47edb..2655a2772 100644
--- a/registry/servicediscovery/store/cache_manager.go
+++ b/registry/servicediscovery/store/cache_manager.go
@@ -27,7 +27,7 @@ import (
import (
"github.com/dubbogo/gost/log/logger"
- "github.com/hashicorp/golang-lru"
+ lru "github.com/hashicorp/golang-lru"
)
type CacheManager struct {
@@ -108,6 +108,7 @@ func (cm *CacheManager) loadCache() error {
if err != nil {
return err
}
+ defer cf.Close()
decoder := gob.NewDecoder(cf)
for {
@@ -123,7 +124,7 @@ func (cm *CacheManager) loadCache() error {
cm.cache.Add(it.Key, it.Value)
}
- return cf.Close()
+ return nil
}
// dumpCache dumps the cache to the cache file.
diff --git a/registry/servicediscovery/store/cache_manager_fd_test.go
b/registry/servicediscovery/store/cache_manager_fd_test.go
new file mode 100644
index 000000000..7c59e1c38
--- /dev/null
+++ b/registry/servicediscovery/store/cache_manager_fd_test.go
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package store
+
+import (
+ "os"
+ "path/filepath"
+ "runtime"
+ "testing"
+)
+
+import (
+ lru "github.com/hashicorp/golang-lru"
+)
+
+func TestLoadCacheClosesFileOnDecodeError(t *testing.T) {
+ if runtime.GOOS != "linux" {
+ t.Skip("requires Linux /proc fd accounting")
+ }
+
+ cacheFile := filepath.Join(t.TempDir(), "cache")
+ if err := os.WriteFile(cacheFile, []byte("corrupt gob"), 0o600); err !=
nil {
+ t.Fatal(err)
+ }
+ cache, err := lru.New(10)
+ if err != nil {
+ t.Fatal(err)
+ }
+ cm := &CacheManager{cacheFile: cacheFile, cache: cache}
+
+ before := countOpenFiles(t)
+ for range 128 {
+ if err := cm.loadCache(); err == nil {
+ t.Fatal("loadCache should return the decoder error")
+ }
+ }
+ after := countOpenFiles(t)
+ if after > before+2 {
+ t.Fatalf("open file descriptors grew from %d to %d", before,
after)
+ }
+}
+
+func countOpenFiles(t *testing.T) int {
+ t.Helper()
+ entries, err := os.ReadDir("/proc/self/fd")
+ if err != nil {
+ t.Fatal(err)
+ }
+ return len(entries)
+}
