Skip to content

Conversation

zeripath
Copy link
Contributor

There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

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

There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

Signed-off-by: Andrew Thornton <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #19537 (520b20a) into main (ebb2396) will decrease coverage by 0.06%.
The diff coverage is 41.46%.

@@            Coverage Diff             @@
##             main   #19537      +/-   ##
==========================================
- Coverage   47.42%   47.36%   -0.07%     
==========================================
  Files         951      950       -1     
  Lines      132360   132332      -28     
==========================================
- Hits        62767    62674      -93     
- Misses      62032    62099      +67     
+ Partials     7561     7559       -2     
Impacted Files Coverage Δ
cmd/dump.go 0.70% <0.00%> (-0.01%) ⬇️
models/issue_label.go 68.46% <0.00%> (+0.44%) ⬆️
modules/context/api.go 67.47% <0.00%> (-0.28%) ⬇️
modules/queue/workerpool.go 52.59% <ø> (-2.08%) ⬇️
routers/api/v1/repo/repo.go 65.30% <ø> (ø)
routers/web/events/events.go 0.00% <ø> (ø)
routers/web/repo/cherry_pick.go 0.00% <0.00%> (ø)
routers/web/repo/commit.go 25.53% <0.00%> (-0.64%) ⬇️
routers/web/repo/issue_stopwatch.go 39.24% <0.00%> (-6.35%) ⬇️
routers/web/repo/repo.go 24.32% <0.00%> (-0.11%) ⬇️
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 960b813...520b20a. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 28, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 28, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 28, 2022
@wxiaoguang wxiaoguang merged commit 332b2ec into go-gitea:main Apr 28, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 28, 2022
* giteaofficial/main: (21 commits)
  Prevent intermittent race in attribute reader close (go-gitea#19537)
  Make repository file list useable on mobile (go-gitea#19515)
  Update image URL for Discord webhook (go-gitea#19536)
  [skip ci] Updated translations via Crowdin
  Fix 64-bit atomic operations on 32-bit machines (go-gitea#19531)
  Fix `upgrade.sh` script error with `su -c` (go-gitea#19483)
  When view _Siderbar or _Footer, just display once (go-gitea#19501)
  Fix migrate release from github (go-gitea#19510)
  Prevent dangling archiver goroutine (go-gitea#19516)
  Don't let repo clone URL overflow (go-gitea#19517)
  Add commit status popup to issuelist (go-gitea#19375)
  Disable unnecessary GitHooks elements (go-gitea#18485)
  Disable unnecessary GitHooks elements
  Improve dashboard's repo list performance (go-gitea#18963)
  By default force vertical tabs on mobile (go-gitea#19486)
  Refactor readme file renderer (go-gitea#19502)
  Allow package dump skipping (go-gitea#19506)
  Unset git author/committer variables when running integration tests (go-gitea#19512)
  Allow commit status popup on /pulls page (go-gitea#19507)
  Use router param for filepath in GetRawFile (go-gitea#19499)
  ...
@zeripath zeripath deleted the prevent-intermittent-race-in-close-gitattributes branch April 28, 2022 09:15
zeripath added a commit to zeripath/gitea that referenced this pull request Apr 28, 2022
Backport go-gitea#19537

There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added the backport/done All backports for this PR have been created label Apr 28, 2022
6543 pushed a commit that referenced this pull request Apr 28, 2022
Backport #19537

There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

Signed-off-by: Andrew Thornton <[email protected]>
Codeberg-org pushed a commit to Codeberg-org/gitea that referenced this pull request Apr 28, 2022
…o-gitea#19539)

Backport go-gitea#19537

There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

Signed-off-by: Andrew Thornton <[email protected]>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

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

Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: 6543 <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants