Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Fixed Race Condition #453 #30

Closed

Conversation

kammerdiener
Copy link

Fixed how it is doing the this._getDBs and this._putDBs

PR #28 fixes the this._getDBs issue and this has the same fix.

Fixed how it is doing the this._getDBs and this.putDBs
@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.149% when pulling c40a3e5 on kammerdiener:Race-Condition-Fix-For-#453 into d3baf12 on ethereumjs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.149% when pulling a3be59a on kammerdiener:Race-Condition-Fix-For-#453 into d3baf12 on ethereumjs:master.

Copy link

@Asone Asone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gived a look about the PR and compared to this PR :

https://github.com/ethereumjs/merkle-patricia-tree/pull/28/files

which is similar. This current PR has a single line of difference for which i'm asking myself of potential typo.

Sorry if i'm wrong. Anyway, solid knowledge from devs to handle that kind of stuff.

Much #datalove

this.__putDBs = this._putDBs
this._putDBs = [this._scratch]
this._putDBs = [this._scratch].concat(this._getDBs)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't know much about the code around this, but 've been reading the the related issues that led to this PR ( originally reading from the race conditions issue opened in ganache-cli repo ).

as _putDBs and _getDBs seems to be different objects, is op ( @kammerdiener ) sure that this is not a typo ?

Was this._putDBs = [this._scratch].concat(this._putDBs); intended instead ?

( sorry if it is not, just trying to educate myself ^^" )

@holgerd77
Copy link
Member

Sorry, this was left aside a bit. Will have a look in the upcoming days (maybe next week).

@holgerd77 holgerd77 self-assigned this Mar 5, 2018
@holgerd77
Copy link
Member

Thanks for the PR! Since there was a fix on this submitted earlier and also having a cleaner commit history, I will close here.

@holgerd77 holgerd77 closed this Mar 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants