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

fix(nodebuilder): fix nil pointer exception and simplify synchronization #2466

Merged
merged 4 commits into from
Jul 13, 2023

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Jul 12, 2023

We hit an infamous use case here where nil is not nil. Also, I simplified locking here while thinking that's a race condition.

@codecov-commenter
Copy link

Codecov Report

Merging #2466 (42ce45b) into main (c8fa853) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2466      +/-   ##
==========================================
+ Coverage   53.05%   53.12%   +0.06%     
==========================================
  Files         156      156              
  Lines        9940     9935       -5     
==========================================
+ Hits         5274     5278       +4     
+ Misses       4210     4202       -8     
+ Partials      456      455       -1     
Impacted Files Coverage Δ
nodebuilder/store.go 65.68% <100.00%> (-0.67%) ⬇️

... and 5 files with indirect coverage changes

@Wondertan Wondertan changed the title fix(nodebuilder): fsStore synchronize on Close fix(nodebuilder): fix nil pointer exception and simplify synchronization Jul 12, 2023
@Wondertan Wondertan self-assigned this Jul 12, 2023
@Wondertan Wondertan added the kind:fix Attached to bug-fixing PRs label Jul 12, 2023
@Wondertan Wondertan marked this pull request as ready for review July 12, 2023 17:10
@Wondertan Wondertan enabled auto-merge (squash) July 12, 2023 17:10
vgonkivs
vgonkivs previously approved these changes Jul 13, 2023
renaynay
renaynay previously approved these changes Jul 13, 2023
@renaynay renaynay disabled auto-merge July 13, 2023 09:36
@Wondertan Wondertan dismissed stale reviews from renaynay and vgonkivs via acf7208 July 13, 2023 09:53
@Wondertan Wondertan enabled auto-merge (squash) July 13, 2023 11:55
@Wondertan Wondertan merged commit 85cec8e into main Jul 13, 2023
27 of 33 checks passed
@Wondertan Wondertan deleted the hlib/store/lock-fix branch July 13, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:fix Attached to bug-fixing PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants