Skip to content

Fix the potential risk to get null CurrentHeaderHash from Blockchain (2x)#1030

Merged
vncoelho merged 2 commits intoneo-project:master-2.xfrom
yongjiema:fix-blockchain-currentheaderhash-null
Aug 16, 2019
Merged

Fix the potential risk to get null CurrentHeaderHash from Blockchain (2x)#1030
vncoelho merged 2 commits intoneo-project:master-2.xfrom
yongjiema:fix-blockchain-currentheaderhash-null

Conversation

@yongjiema
Copy link
Copy Markdown
Contributor

No description provided.

@erikzhang
Copy link
Copy Markdown
Member

What's the difference?

@yongjiema yongjiema changed the title Fix the potential risk to get null CurrentHeaderHash from Blockchain Fix the potential risk to get null CurrentHeaderHash from Blockchain (2x) Aug 15, 2019
@yongjiema
Copy link
Copy Markdown
Contributor Author

yongjiema commented Aug 15, 2019

What's the difference?

Before: Reading without lock may get null value from a changeable collection in case of multiple reading, please check https://stackoverflow.com/questions/17769892/c-sharp-multi-threaded-queue-items-are-null for more details.

After: The Interlocked.Exchange is an atomic operation which is friendly for multithreading (https://github.com/neo-project/neo/blob/master-2.x/neo/Ledger/Blockchain.cs#L729).

PS: I've encountered the issue on my local node.

@vncoelho vncoelho added the Port-to-3.x Feature or PR must be ported to Neo 3.x branch label Aug 15, 2019
Copy link
Copy Markdown
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Tested and working normally. I did not reproduce the fix, but makes sense.

Nice job you are doing @yongjiema, let's port this to 3x, right?

@yongjiema
Copy link
Copy Markdown
Contributor Author

@vncoelho This fix is for 2x, I've just created a PR #1033 for 3x.

@vncoelho vncoelho reopened this Aug 16, 2019
@vncoelho vncoelho merged commit 56174b8 into neo-project:master-2.x Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Port-to-3.x Feature or PR must be ported to Neo 3.x branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants