Skip to content

Conversation

zeripath
Copy link
Contributor

Because of reuse of the old paused/resumed channels in this test there
was a potential for deadlock. This PR ensures that the channels are always
reobtained.

It further adds some control code to detect hangs in future - and it
ensures that the pausing warning is not shown on shutdown.

Signed-off-by: Andrew Thornton art27@cantab.net

Because of reuse of the old paused/resumed channels in this test there
was a potential for deadlock. This PR ensures that the channels are always
reobtained.

It further adds some control code to detect hangs in future - and it
ensures that the pausing warning is not shown on shutdown.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added type/bug type/testing skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Jan 25, 2022
@zeripath zeripath added this to the 1.17.0 milestone Jan 25, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 25, 2022
@Gusted
Copy link
Contributor

Gusted commented Jan 25, 2022

CI error is related:

--- FAIL: TestPersistableChannelQueue_Pause (0.95s)

    queue_disk_channel_test.go:470: 

        	Error Trace:	queue_disk_channel_test.go:470

        	Error:      	Handler processing should have stopped

        	Test:       	TestPersistableChannelQueue_Pause

@zeripath
Copy link
Contributor Author

ugh I hate these tests.

At least it's not deadlocking...

Signed-off-by: Andrew Thornton <art27@cantab.net>
@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 Jan 25, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #18410 (4f0979a) into main (80adbeb) will increase coverage by 0.02%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18410      +/-   ##
==========================================
+ Coverage   46.02%   46.04%   +0.02%     
==========================================
  Files         840      840              
  Lines       92925    92928       +3     
==========================================
+ Hits        42772    42793      +21     
+ Misses      43363    43343      -20     
- Partials     6790     6792       +2     
Impacted Files Coverage Δ
modules/queue/workerpool.go 55.62% <33.33%> (-2.11%) ⬇️
models/repo_list.go 76.62% <0.00%> (-0.60%) ⬇️
services/pull/pull.go 42.39% <0.00%> (-0.41%) ⬇️
models/issue_comment.go 55.14% <0.00%> (+0.28%) ⬆️
modules/log/file.go 75.20% <0.00%> (+1.60%) ⬆️
modules/queue/queue_channel.go 82.60% <0.00%> (+3.26%) ⬆️
modules/queue/queue_disk.go 69.38% <0.00%> (+4.08%) ⬆️
modules/queue/queue_bytefifo.go 59.54% <0.00%> (+9.54%) ⬆️

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 80adbeb...4f0979a. Read the comment docs.

@lafriks
Copy link
Member

lafriks commented Jan 25, 2022

🚀

@lafriks lafriks merged commit 713985b into go-gitea:main Jan 25, 2022
@zeripath zeripath deleted the prevent-deadlocks-in-pause-test branch January 25, 2022 23:19
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 26, 2022
* giteaofficial/main:
  Use base32 for 2FA scratch token (go-gitea#18384)
  [skip ci] Updated translations via Crowdin
  Fix broken oauth2 authentication source edit page (go-gitea#18412)
  Prevent deadlocks in persistable channel pause test (go-gitea#18410)
  Bump golangci-lint version (go-gitea#18411)
  Unexport git.GlobalCommandArgs (go-gitea#18376)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
* Prevent deadlocks in persistable channel pause test

Because of reuse of the old paused/resumed channels in this test there
was a potential for deadlock. This PR ensures that the channels are always
reobtained.

It further adds some control code to detect hangs in future - and it
ensures that the pausing warning is not shown on shutdown.

Signed-off-by: Andrew Thornton <art27@cantab.net>

* do not warn but do pause

Signed-off-by: Andrew Thornton <art27@cantab.net>
@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/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug type/testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants