Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Conversation

@alcortesm
Copy link
Contributor

Commit.File() was leaking a goroutine because it was looping over
an iterator without closing its channel.

Now commit.File() calls the new Tree.File() method that searches
the file in the repository by trasversing the dir tree instead
of using the tree.Files() iterator.

This not only prevent the goroutine leak, but also speeds up
file searching.

Commit.File() was leaking a goroutine because it was looping over
an iterator without closing its channel.

Now commit.File() calls the new Tree.File() method that searches
the file in the repository by trasversing the dir tree instead
of using the tree.Files() iterator.

This not only prevent the goroutine leak, but also speeds up
file searching.
@codecov-io
Copy link

Current coverage is 69.51%

Merging #14 into master will increase coverage by +1.14% as of 827470d

@@            master     #14   diff @@
======================================
  Files           17      17       
  Stmts         1208    1253    +45
  Branches       192     200     +8
  Methods          0       0       
======================================
+ Hit            826     871    +45
  Partial         93      93       
  Missed         289     289       

Review entire Coverage Diff as of 827470d

Powered by Codecov. Updated on successful CI builds.

tree.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a public method, it needs a comment 🎉

@toqueteos
Copy link
Contributor

LGTM

mcuadros added a commit that referenced this pull request Jan 21, 2016
@mcuadros mcuadros merged commit 9f933cb into src-d:master Jan 21, 2016
@alcortesm alcortesm deleted the fix-file-iterator-gorutine-leak branch January 21, 2016 12:10
mcuadros added a commit that referenced this pull request Jan 31, 2017
gsalingu-ovhus pushed a commit to gsalingu-ovhus/go-git that referenced this pull request Mar 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants