Skip to content

Prevent dangling cat-files (#17154) #17155

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

Closed
wants to merge 7 commits into from

Conversation

zeripath
Copy link
Contributor

Backport #17154

In #17138 it appears that on Windows a git cat-file can fail to be killed.

This PR attempts to prevent this by:

  1. Using os.Pipes for the input - this should mean that the stdin is definitely closed.
  2. If the cat-file is doesn't close properly it will attempt to kill it repeatedly logging this.

Fix #17138

Signed-off-by: Andrew Thornton [email protected]

Backport go-gitea#17154

In go-gitea#17138 it appears that on Windows a git cat-file can fail to be killed.

This PR attempts to prevent this by:

1. Using os.Pipes for the input - this should mean that the stdin is definitely closed.
2. If the cat-file is doesn't close properly it will attempt to kill it repeatedly logging this.

Fix go-gitea#17138

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added type/bug pr/wip This PR is not ready for review labels Sep 25, 2021
@zeripath zeripath added this to the 1.15.4 milestone Sep 25, 2021
@zeripath
Copy link
Contributor Author

WIP because the current mechanism is racy

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 25, 2021
@techknowlogick techknowlogick removed this from the 1.15.4 milestone Oct 8, 2021
@zeripath zeripath closed this Nov 3, 2021
@zeripath zeripath deleted the backport-17154 branch November 3, 2021 07:12
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants