From 0ead624473b6bddc232b46bc7c76ab4f9c743ff5 Mon Sep 17 00:00:00 2001 From: allencloud Date: Sat, 25 Jun 2016 11:57:21 +0800 Subject: [PATCH] add defer file.Close to avoid potential fd leak Signed-off-by: allencloud --- api/client/image/build.go | 1 + builder/dockerfile/internals.go | 4 ++- builder/dockerfile/parser/dumper/main.go | 1 + builder/dockerfile/parser/parser_test.go | 3 +-- builder/dockerignore.go | 1 + builder/dockerignore/dockerignore.go | 4 +-- builder/dockerignore/dockerignore_test.go | 2 ++ cliconfig/config_test.go | 4 +++ daemon/logger/jsonfilelog/read.go | 3 +++ image/tarexport/save.go | 26 ++++++++++--------- integration-cli/docker_cli_daemon_test.go | 4 +++ integration-cli/docker_cli_push_test.go | 1 + integration-cli/docker_cli_save_load_test.go | 1 + .../docker_cli_save_load_unix_test.go | 1 + integration-cli/registry.go | 2 ++ integration-cli/requirements.go | 1 + integration-cli/trust_server.go | 5 ++-- layer/layer_test.go | 6 +++-- libcontainerd/remote_linux.go | 6 ++--- pkg/archive/archive_test.go | 22 ++++++++++++++++ pkg/tarsum/builder_context_test.go | 4 +++ pkg/tarsum/tarsum_test.go | 12 +++++++-- plugin/backend.go | 2 ++ plugin/manager.go | 1 + profiles/apparmor/apparmor.go | 2 ++ 25 files changed, 93 insertions(+), 26 deletions(-) diff --git a/api/client/image/build.go b/api/client/image/build.go index 38f4bcd439..8cc0736446 100644 --- a/api/client/image/build.go +++ b/api/client/image/build.go @@ -162,6 +162,7 @@ func runBuild(dockerCli *client.DockerCli, options buildOptions) error { if err != nil && !os.IsNotExist(err) { return err } + defer f.Close() var excludes []string if err == nil { diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 2dc3fe6bd1..7805798461 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -255,9 +255,9 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) { // ignoring error because the file was already opened successfully tmpFileSt, err := tmpFile.Stat() if err != nil { + tmpFile.Close() return } - tmpFile.Close() // Set the mtime to the Last-Modified header value if present // Otherwise just remove atime and mtime @@ -272,6 +272,8 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) { } } + tmpFile.Close() + if err = system.Chtimes(tmpFileName, mTime, mTime); err != nil { return } diff --git a/builder/dockerfile/parser/dumper/main.go b/builder/dockerfile/parser/dumper/main.go index 6561708c23..fff3046fd3 100644 --- a/builder/dockerfile/parser/dumper/main.go +++ b/builder/dockerfile/parser/dumper/main.go @@ -21,6 +21,7 @@ func main() { if err != nil { panic(err) } + defer f.Close() d := parser.Directive{LookingForDirectives: true} parser.SetEscapeToken(parser.DefaultEscapeToken, &d) diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go index e7b0c07c53..e8e26961de 100644 --- a/builder/dockerfile/parser/parser_test.go +++ b/builder/dockerfile/parser/parser_test.go @@ -38,6 +38,7 @@ func TestTestNegative(t *testing.T) { if err != nil { t.Fatalf("Dockerfile missing for %s: %v", dir, err) } + defer df.Close() d := Directive{LookingForDirectives: true} SetEscapeToken(DefaultEscapeToken, &d) @@ -45,8 +46,6 @@ func TestTestNegative(t *testing.T) { if err == nil { t.Fatalf("No error parsing broken dockerfile for %s", dir) } - - df.Close() } } diff --git a/builder/dockerignore.go b/builder/dockerignore.go index 2990770a4a..3da7913367 100644 --- a/builder/dockerignore.go +++ b/builder/dockerignore.go @@ -36,6 +36,7 @@ func (c DockerIgnoreContext) Process(filesToRemove []string) error { return err } excludes, _ := dockerignore.ReadAll(f) + f.Close() filesToRemove = append([]string{".dockerignore"}, filesToRemove...) for _, fileToRemove := range filesToRemove { rm, _ := fileutils.Matches(fileToRemove, excludes) diff --git a/builder/dockerignore/dockerignore.go b/builder/dockerignore/dockerignore.go index 9ddf5dd51e..2db67be799 100644 --- a/builder/dockerignore/dockerignore.go +++ b/builder/dockerignore/dockerignore.go @@ -12,11 +12,11 @@ import ( // ReadAll reads a .dockerignore file and returns the list of file patterns // to ignore. Note this will trim whitespace from each line as well // as use GO's "clean" func to get the shortest/cleanest path for each. -func ReadAll(reader io.ReadCloser) ([]string, error) { +func ReadAll(reader io.Reader) ([]string, error) { if reader == nil { return nil, nil } - defer reader.Close() + scanner := bufio.NewScanner(reader) var excludes []string currentLine := 0 diff --git a/builder/dockerignore/dockerignore_test.go b/builder/dockerignore/dockerignore_test.go index 361b041912..612a1399cd 100644 --- a/builder/dockerignore/dockerignore_test.go +++ b/builder/dockerignore/dockerignore_test.go @@ -35,6 +35,8 @@ func TestReadAll(t *testing.T) { if err != nil { t.Fatal(err) } + defer diFd.Close() + di, err = ReadAll(diFd) if err != nil { t.Fatal(err) diff --git a/cliconfig/config_test.go b/cliconfig/config_test.go index 78b360cc2c..6eff26e481 100644 --- a/cliconfig/config_test.go +++ b/cliconfig/config_test.go @@ -484,6 +484,8 @@ func TestJsonSaveWithNoFile(t *testing.T) { fn := filepath.Join(tmpHome, ConfigFileName) f, _ := os.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + defer f.Close() + err = config.SaveToWriter(f) if err != nil { t.Fatalf("Failed saving to file: %q", err) @@ -522,6 +524,8 @@ func TestLegacyJsonSaveWithNoFile(t *testing.T) { fn := filepath.Join(tmpHome, ConfigFileName) f, _ := os.OpenFile(fn, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + defer f.Close() + if err = config.SaveToWriter(f); err != nil { t.Fatalf("Failed saving to file: %q", err) } diff --git a/daemon/logger/jsonfilelog/read.go b/daemon/logger/jsonfilelog/read.go index bea83ddf14..9e851eaa8f 100644 --- a/daemon/logger/jsonfilelog/read.go +++ b/daemon/logger/jsonfilelog/read.go @@ -55,6 +55,8 @@ func (l *JSONFileLogger) readLogs(logWatcher *logger.LogWatcher, config logger.R } continue } + defer f.Close() + files = append(files, f) } @@ -63,6 +65,7 @@ func (l *JSONFileLogger) readLogs(logWatcher *logger.LogWatcher, config logger.R logWatcher.Err <- err return } + defer latestFile.Close() if config.Tail != 0 { tailer := ioutils.MultiReadSeeker(append(files, latestFile)...) diff --git a/image/tarexport/save.go b/image/tarexport/save.go index ddb87ee954..5f1bd7f1ce 100644 --- a/image/tarexport/save.go +++ b/image/tarexport/save.go @@ -174,17 +174,18 @@ func (s *saveSession) save(outStream io.Writer) error { if len(reposLegacy) > 0 { reposFile := filepath.Join(tempDir, legacyRepositoriesFileName) - f, err := os.OpenFile(reposFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) + rf, err := os.OpenFile(reposFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) if err != nil { - f.Close() return err } - if err := json.NewEncoder(f).Encode(reposLegacy); err != nil { - return err - } - if err := f.Close(); err != nil { + + if err := json.NewEncoder(rf).Encode(reposLegacy); err != nil { + rf.Close() return err } + + rf.Close() + if err := system.Chtimes(reposFile, time.Unix(0, 0), time.Unix(0, 0)); err != nil { return err } @@ -193,15 +194,16 @@ func (s *saveSession) save(outStream io.Writer) error { manifestFileName := filepath.Join(tempDir, manifestFileName) f, err := os.OpenFile(manifestFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) if err != nil { + return err + } + + if err := json.NewEncoder(f).Encode(manifest); err != nil { f.Close() return err } - if err := json.NewEncoder(f).Encode(manifest); err != nil { - return err - } - if err := f.Close(); err != nil { - return err - } + + f.Close() + if err := system.Chtimes(manifestFileName, time.Unix(0, 0), time.Unix(0, 0)); err != nil { return err } diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index a35bfcd41d..93f0129d91 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -1151,6 +1151,8 @@ func (s *DockerDaemonSuite) TestDaemonLoggingDriverDefault(c *check.C) { if err != nil { c.Fatal(err) } + defer f.Close() + var res struct { Log string `json:"log"` Stream string `json:"stream"` @@ -1229,6 +1231,8 @@ func (s *DockerDaemonSuite) TestDaemonLoggingDriverNoneOverride(c *check.C) { if err != nil { c.Fatal(err) } + defer f.Close() + var res struct { Log string `json:"log"` Stream string `json:"stream"` diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go index f9d53449fb..7c14824777 100644 --- a/integration-cli/docker_cli_push_test.go +++ b/integration-cli/docker_cli_push_test.go @@ -131,6 +131,7 @@ func testPushEmptyLayer(c *check.C) { freader, err := os.Open(emptyTarball.Name()) c.Assert(err, check.IsNil, check.Commentf("Could not open test tarball")) + defer freader.Close() importCmd := exec.Command(dockerBinary, "import", "-", repoName) importCmd.Stdin = freader diff --git a/integration-cli/docker_cli_save_load_test.go b/integration-cli/docker_cli_save_load_test.go index 869bf6bf88..70139a59bc 100644 --- a/integration-cli/docker_cli_save_load_test.go +++ b/integration-cli/docker_cli_save_load_test.go @@ -283,6 +283,7 @@ func (s *DockerSuite) TestSaveDirectoryPermissions(c *check.C) { f, err := os.Open(layerPath) c.Assert(err, checker.IsNil, check.Commentf("failed to open %s: %s", layerPath, err)) + defer f.Close() entries, err := listTar(f) for _, e := range entries { diff --git a/integration-cli/docker_cli_save_load_unix_test.go b/integration-cli/docker_cli_save_load_unix_test.go index d9dd95f126..7cc1013432 100644 --- a/integration-cli/docker_cli_save_load_unix_test.go +++ b/integration-cli/docker_cli_save_load_unix_test.go @@ -35,6 +35,7 @@ func (s *DockerSuite) TestSaveAndLoadRepoStdout(c *check.C) { tmpFile, err = os.Open(tmpFile.Name()) c.Assert(err, check.IsNil) + defer tmpFile.Close() deleteImages(repoName) diff --git a/integration-cli/registry.go b/integration-cli/registry.go index e3e16f1cfb..5181570af5 100644 --- a/integration-cli/registry.go +++ b/integration-cli/registry.go @@ -76,6 +76,8 @@ http: if err != nil { return nil, err } + defer config.Close() + if _, err := fmt.Fprintf(config, template, tmp, privateRegistryURL, authTemplate); err != nil { os.RemoveAll(tmp) return nil, err diff --git a/integration-cli/requirements.go b/integration-cli/requirements.go index d475892107..a9fe468e47 100644 --- a/integration-cli/requirements.go +++ b/integration-cli/requirements.go @@ -175,6 +175,7 @@ var ( // We need extra check on redhat based distributions if f, err := os.Open("/sys/module/user_namespace/parameters/enable"); err == nil { + defer f.Close() b := make([]byte, 1) _, _ = f.Read(b) if string(b) == "N" { diff --git a/integration-cli/trust_server.go b/integration-cli/trust_server.go index 7ed61fffb7..72c474bbbc 100644 --- a/integration-cli/trust_server.go +++ b/integration-cli/trust_server.go @@ -61,10 +61,10 @@ func newTestNotary(c *check.C) (*testNotary, error) { } confPath := filepath.Join(tmp, "config.json") config, err := os.Create(confPath) - defer config.Close() if err != nil { return nil, err } + defer config.Close() workingDir, err := os.Getwd() if err != nil { @@ -78,10 +78,11 @@ func newTestNotary(c *check.C) (*testNotary, error) { // generate client config clientConfPath := filepath.Join(tmp, "client-config.json") clientConfig, err := os.Create(clientConfPath) - defer clientConfig.Close() if err != nil { return nil, err } + defer clientConfig.Close() + template = `{ "trust_dir" : "%s", "remote_server": { diff --git a/layer/layer_test.go b/layer/layer_test.go index 8e6817c96a..9df71b92a6 100644 --- a/layer/layer_test.go +++ b/layer/layer_test.go @@ -741,18 +741,20 @@ func TestTarStreamVerification(t *testing.T) { if err != nil { t.Fatal(err) } + defer src.Close() dst, err := os.Create(filepath.Join(tmpdir, id2.Algorithm().String(), id2.Hex(), "tar-split.json.gz")) if err != nil { t.Fatal(err) } + defer dst.Close() if _, err := io.Copy(dst, src); err != nil { t.Fatal(err) } - src.Close() - dst.Close() + src.Sync() + dst.Sync() ts, err := layer2.TarStream() if err != nil { diff --git a/libcontainerd/remote_linux.go b/libcontainerd/remote_linux.go index 57daa7fc7b..7afe0da399 100644 --- a/libcontainerd/remote_linux.go +++ b/libcontainerd/remote_linux.go @@ -216,11 +216,11 @@ func (r *remote) Client(b Backend) (Client, error) { func (r *remote) updateEventTimestamp(t time.Time) { f, err := os.OpenFile(r.eventTsPath, syscall.O_CREAT|syscall.O_WRONLY|syscall.O_TRUNC, 0600) - defer f.Close() if err != nil { logrus.Warnf("libcontainerd: failed to open event timestamp file: %v", err) return } + defer f.Close() b, err := t.MarshalText() if err != nil { @@ -245,11 +245,11 @@ func (r *remote) getLastEventTimestamp() time.Time { } f, err := os.Open(r.eventTsPath) - defer f.Close() if err != nil { logrus.Warnf("libcontainerd: Unable to access last event ts: %v", err) return t } + defer f.Close() b := make([]byte, fi.Size()) n, err := f.Read(b) @@ -329,10 +329,10 @@ func (r *remote) handleEventStream(events containerd.API_EventsClient) { func (r *remote) runContainerdDaemon() error { pidFilename := filepath.Join(r.stateDir, containerdPidFilename) f, err := os.OpenFile(pidFilename, os.O_RDWR|os.O_CREATE, 0600) - defer f.Close() if err != nil { return err } + defer f.Close() // File exist, check if the daemon is alive b := make([]byte, 8) diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index 85e41227c0..05db3d7ed9 100644 --- a/pkg/archive/archive_test.go +++ b/pkg/archive/archive_test.go @@ -101,6 +101,11 @@ func TestDecompressStreamGzip(t *testing.T) { t.Fatalf("Fail to create an archive file for test : %s.", output) } archive, err := os.Open(tmp + "archive.gz") + if err != nil { + t.Fatalf("Fail to open file archive.gz") + } + defer archive.Close() + _, err = DecompressStream(archive) if err != nil { t.Fatalf("Failed to decompress a gzip file.") @@ -114,6 +119,11 @@ func TestDecompressStreamBzip2(t *testing.T) { t.Fatalf("Fail to create an archive file for test : %s.", output) } archive, err := os.Open(tmp + "archive.bz2") + if err != nil { + t.Fatalf("Fail to open file archive.bz2") + } + defer archive.Close() + _, err = DecompressStream(archive) if err != nil { t.Fatalf("Failed to decompress a bzip2 file.") @@ -130,6 +140,10 @@ func TestDecompressStreamXz(t *testing.T) { t.Fatalf("Fail to create an archive file for test : %s.", output) } archive, err := os.Open(tmp + "archive.xz") + if err != nil { + t.Fatalf("Fail to open file archive.xz") + } + defer archive.Close() _, err = DecompressStream(archive) if err != nil { t.Fatalf("Failed to decompress an xz file.") @@ -141,6 +155,8 @@ func TestCompressStreamXzUnsuported(t *testing.T) { if err != nil { t.Fatalf("Fail to create the destination file") } + defer dest.Close() + _, err = CompressStream(dest, Xz) if err == nil { t.Fatalf("Should fail as xz is unsupported for compression format.") @@ -152,6 +168,8 @@ func TestCompressStreamBzip2Unsupported(t *testing.T) { if err != nil { t.Fatalf("Fail to create the destination file") } + defer dest.Close() + _, err = CompressStream(dest, Xz) if err == nil { t.Fatalf("Should fail as xz is unsupported for compression format.") @@ -163,6 +181,8 @@ func TestCompressStreamInvalid(t *testing.T) { if err != nil { t.Fatalf("Fail to create the destination file") } + defer dest.Close() + _, err = CompressStream(dest, -1) if err == nil { t.Fatalf("Should fail as xz is unsupported for compression format.") @@ -795,6 +815,8 @@ func TestUntarUstarGnuConflict(t *testing.T) { if err != nil { t.Fatal(err) } + defer f.Close() + found := false tr := tar.NewReader(f) // Iterate through the files in the archive. diff --git a/pkg/tarsum/builder_context_test.go b/pkg/tarsum/builder_context_test.go index 719f72895d..f54bf3a1bd 100644 --- a/pkg/tarsum/builder_context_test.go +++ b/pkg/tarsum/builder_context_test.go @@ -14,6 +14,8 @@ func TestTarSumRemoveNonExistent(t *testing.T) { if err != nil { t.Fatal(err) } + defer reader.Close() + ts, err := NewTarSum(reader, false, Version0) if err != nil { t.Fatal(err) @@ -42,6 +44,8 @@ func TestTarSumRemove(t *testing.T) { if err != nil { t.Fatal(err) } + defer reader.Close() + ts, err := NewTarSum(reader, false, Version0) if err != nil { t.Fatal(err) diff --git a/pkg/tarsum/tarsum_test.go b/pkg/tarsum/tarsum_test.go index 54bec53fc9..86df0e2b89 100644 --- a/pkg/tarsum/tarsum_test.go +++ b/pkg/tarsum/tarsum_test.go @@ -200,6 +200,8 @@ func TestNewTarSumForLabel(t *testing.T) { if err != nil { t.Fatal(err) } + defer reader.Close() + label := strings.Split(layer.tarsum, ":")[0] ts, err := NewTarSumForLabel(reader, false, label) if err != nil { @@ -302,6 +304,8 @@ func TestTarSumsReadSize(t *testing.T) { if err != nil { t.Fatal(err) } + defer reader.Close() + ts, err := NewTarSum(reader, false, layer.version) if err != nil { t.Fatal(err) @@ -380,6 +384,8 @@ func TestTarSums(t *testing.T) { t.Errorf("failed to open %s: %s", layer.jsonfile, err) continue } + defer jfh.Close() + buf, err := ioutil.ReadAll(jfh) if err != nil { t.Errorf("failed to readAll %s: %s", layer.jsonfile, err) @@ -559,12 +565,13 @@ func Benchmark9kTar(b *testing.B) { b.Error(err) return } + defer fh.Close() + n, err := io.Copy(buf, fh) if err != nil { b.Error(err) return } - fh.Close() reader := bytes.NewReader(buf.Bytes()) @@ -589,12 +596,13 @@ func Benchmark9kTarGzip(b *testing.B) { b.Error(err) return } + defer fh.Close() + n, err := io.Copy(buf, fh) if err != nil { b.Error(err) return } - fh.Close() reader := bytes.NewReader(buf.Bytes()) diff --git a/plugin/backend.go b/plugin/backend.go index 5b2d9f46a6..89458f59d3 100644 --- a/plugin/backend.go +++ b/plugin/backend.go @@ -124,6 +124,8 @@ func (pm *Manager) Push(name string, metaHeader http.Header, authConfig *types.A if err != nil { return err } + defer rootfs.Close() + _, err = distribution.Push(name, pm.registryService, metaHeader, authConfig, config, rootfs) // XXX: Ignore returning digest for now. // Since digest needs to be written to the ProgressWriter. diff --git a/plugin/manager.go b/plugin/manager.go index 2dcc76f111..f28c5a6ed0 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -299,6 +299,7 @@ func (pm *Manager) init() error { } return err } + defer dt.Close() if err := json.NewDecoder(dt).Decode(&pm.plugins); err != nil { return err diff --git a/profiles/apparmor/apparmor.go b/profiles/apparmor/apparmor.go index 51dfa5cf9c..115e43842d 100644 --- a/profiles/apparmor/apparmor.go +++ b/profiles/apparmor/apparmor.go @@ -102,6 +102,8 @@ func IsLoaded(name string) error { if err != nil { return err } + defer file.Close() + r := bufio.NewReader(file) for { p, err := r.ReadString('\n')