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 sha256 race condition panic #9

Merged
merged 1 commit into from
Sep 10, 2020
Merged

Conversation

shaodan
Copy link
Contributor

@shaodan shaodan commented May 18, 2020

No description provided.

@shaodan shaodan mentioned this pull request May 18, 2020
@shaodan
Copy link
Contributor Author

shaodan commented May 24, 2020

Create new hmac instance as needed since it is not goroutine safe

@eynzhang
Copy link
Contributor

eynzhang commented Aug 5, 2020

@shaodan , I tried your test code first, I expect it should cause panic but it didn’t. Can you double check that?

And can you explain how you reproduce the issue #8 ?

@shaodan
Copy link
Contributor Author

shaodan commented Aug 6, 2020

@eynzhang I've fix the panic, so the test should pass. If you want to reproduce that, you need to revert my change of internal/requestbuilder/signer.go

@eynzhang
Copy link
Contributor

@shaodan , I didn’t use your code for test. I used your method TestSigner_Sign_RaceCondition to test against the master branch, and it passed.

@shaodan
Copy link
Contributor Author

shaodan commented Aug 13, 2020

@filinvadim
Copy link

@shaodan @eynzhang Hi folks! Issue was closed but nothing was actually fixed.

@eynzhang eynzhang merged commit 5e9a2e6 into HuobiRDCenter:master Sep 10, 2020
@eynzhang
Copy link
Contributor

@shaodan thanks for your contribution, your request is merged. I am not sure why I can’t reproduce this issue last time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants