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

Engine: Use atomic.LoadInt32/AddInt32 for starting/stopping the database subsystem #412

Merged
merged 1 commit into from Jan 13, 2020

Conversation

@thrasher-
Copy link
Collaborator

thrasher- commented Jan 13, 2020

PR Description

Originally it was possible to crash the bot by sending two subsequent stop requests to the database subsystem (via the cli or an app) due to attempting to close and already closed channel. This method protects the bot when stopping the database subsystem (sets the state to stopping, preventing further stop requests) until the state is finally reset when it does actually shutdown.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested

  • go test ./... -race
  • golangci-lint run
  • Send 2 subsequent stops and you'll get a panic on attempt to close an already closed channel

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally and on Travis with my changes
  • Any dependent changes have been merged and published in downstream modules
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 13, 2020

Codecov Report

Merging #412 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #412      +/-   ##
==========================================
+ Coverage   40.97%   40.98%   +<.01%     
==========================================
  Files         162      162              
  Lines       38575    38579       +4     
==========================================
+ Hits        15807    15810       +3     
+ Misses      21639    21638       -1     
- Partials     1129     1131       +2
Impacted Files Coverage Δ
engine/database.go 0% <0%> (ø) ⬆️
exchanges/lakebtc/lakebtc_websocket.go 61.45% <0%> (+1.67%) ⬆️

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 12159e3...cdb0149. Read the comment docs.

@thrasher- thrasher- changed the title Use atomic.LoadInt32/AddInt32 for database subsystem Engine: Use atomic.LoadInt32/AddInt32 for starting/stopping the database subsystem Jan 13, 2020
Copy link

codelingo bot left a comment

LGTM

@xtda
xtda approved these changes Jan 13, 2020
Copy link
Member

xtda left a comment

tested approved

Copy link
Collaborator

gloriousCode left a comment

tACK! 🦀

Copy link
Collaborator

shazbert left a comment

tACK

@thrasher- thrasher- merged commit 26b8e95 into master Jan 13, 2020
5 of 7 checks passed
5 of 7 checks passed
Travis CI - Branch Build Failed
Details
codecov/patch 0% of diff hit (target 40.97%)
Details
Review Action No issues found.
Details
Travis CI - Pull Request Build Passed
Details
codecov/project 40.98% (+<.01%) compared to 12159e3
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@thrasher- thrasher- deleted the atomic_database branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.