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

Update lunny/levelqueue to prevent NPE when reads are performed after close #20534

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

zeripath
Copy link
Contributor

#20380 reveals that there was a slight bug in the levelqueue code that assumed that
reads and write would not occur after the db is closed. This unfortunately cannot be
assumed, and if a read or write occured this would result in a NPE.

This bug has been fixed in f020868cc2f78a4bb0b110c4c232c74be048453e therefore this
PR updates Gitea to use this.

Fix #20380

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

… close

go-gitea#20380 reveals that there was a slight bug in the levelqueue code that assumed that
reads and write would not occur after the db is closed. This unfortunately cannot be
assumed, and if a read or write occured this would result in a NPE.

This bug has been fixed in f020868cc2f78a4bb0b110c4c232c74be048453e therefore this
PR updates Gitea to use this.

Fix go-gitea#20380

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor Author

I'm not sure that this could actually occur within Gitea itself - certainly it would be very a racy thing that could only occur rarely at shutdown.

However, the bug occurs frequently enough in testing that it should be backported to v1.17.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 29, 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 Jul 29, 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 Jul 29, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@c5bdea9). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main   #20534   +/-   ##
=======================================
  Coverage        ?   46.93%           
=======================================
  Files           ?      978           
  Lines           ?   135461           
  Branches        ?        0           
=======================================
  Hits            ?    63572           
  Misses          ?    64087           
  Partials        ?     7802           

Help us with your feedback. Take ten seconds to tell us how you rate us.

@6543 6543 merged commit 7fe77f0 into go-gitea:main Jul 29, 2022
6543 pushed a commit to 6543-forks/gitea that referenced this pull request Jul 29, 2022
@6543 6543 added dependencies backport/done All backports for this PR have been created labels Jul 29, 2022
@6543
Copy link
Member

6543 commented Jul 29, 2022

-> #20537

lunny pushed a commit that referenced this pull request Jul 29, 2022
… close (#20534) (#20537)

Co-authored-by: zeripath <art27@cantab.net>
@zeripath zeripath deleted the fix-20380-panic-in-tests branch July 29, 2022 13:01
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 1, 2022
* giteaofficial/main: (29 commits)
  [skip ci] Updated translations via Crowdin
  Support localized README (go-gitea#20508)
  Clean up and fix clone button script (go-gitea#20415)
  Add disable download source configuration (go-gitea#20548)
  Fix default merge style (go-gitea#20564)
  Update login methods in package docs (go-gitea#20561)
  Add missing Tabs on organisation/package view (Frontport go-gitea#20539) (go-gitea#20540)
  [skip ci] Updated licenses and gitignores
  Add setting `SQLITE_JOURNAL_MODE` to enable WAL (go-gitea#20535)
  Rework file highlight rendering and fix yaml copy-paste (go-gitea#19967)
  Add new API endpoints for push mirrors management (go-gitea#19841)
  WebAuthn CredentialID field needs to be increased in size (go-gitea#20530)
  Add latest commit's SHA to content response (go-gitea#20398)
  Improve token and secret key generation docs (go-gitea#20387)
  [skip ci] Updated translations via Crowdin
  Rework raw file http header logic (go-gitea#20484)
  Update lunny/levelqueue to prevent NPE when reads are performed after close (go-gitea#20534)
  Added guidance on file to choose to download (go-gitea#20474)
  [skip ci] Updated translations via Crowdin
  Ensure that all unmerged files are merged when conflict checking (go-gitea#20528)
  ...
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
@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/testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI random failure: invalid memory address or nil pointer dereference: leveldb.(*DB).isClosed(...)
6 participants