Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid leaving file handles open #65

Merged
merged 1 commit into from
Apr 28, 2018
Merged

Avoid leaving file handles open #65

merged 1 commit into from
Apr 28, 2018

Conversation

sjp
Copy link
Contributor

@sjp sjp commented Apr 26, 2018

When calculating code coverage for a solution of mine, I found that two projects where reliably stuck on file locking.

The only process using the files were the dotnet process running code coverage reporting. After wrapping disposable objects in using statements, the problem appears to be resolved.

Additionally, I think this may also allow the RetryHelper code to be removed, but this has not been done yet.

@@ -155,7 +154,7 @@ private bool IsBranchTarget(ILProcessor processor, Instruction instruction)
return false;
}

private void ReplaceInstructionTarget(Instruction instruction, Instruction oldTarget, Instruction newTarget)
private static void ReplaceInstructionTarget(Instruction instruction, Instruction oldTarget, Instruction newTarget)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason you made this static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason aside from them not relying on any fields in the class. It's not a functional change in behavior, mostly cosmetic.

@tonerdo tonerdo merged commit 7355d9f into coverlet-coverage:master Apr 28, 2018
NorekZ pushed a commit to NorekZ/coverlet that referenced this pull request Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants