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 session context corrupted problem #349

Merged
merged 2 commits into from
Nov 12, 2023
Merged

Fix session context corrupted problem #349

merged 2 commits into from
Nov 12, 2023

Conversation

PzaThief
Copy link
Contributor

@PzaThief PzaThief commented Nov 9, 2023

Fixes #[ISSUE NUMBER].

No related issue

Changes proposed in this pull request:
First of all, thank you to show nice sample. It was very helpful to build echo project.
But, I found a problem that session in container has shared struct "context".
It makes race condition and cause data error or concurrent map writes.
The first commit is POC of race condition error that never want it to happen.
If remove time.Sleep(1 * time.Second) in second goroutine, you may see also concurrent map writes sometimes.
So, I fixed it to do not use context sharing and use it as parameter.
Now there is a restriction of using session to use echo.context makes can not use in service layer, but there is no reason to access session in business logic, so it it not problem of this structure.
Please comment if you have any question about it.

@ybkuroki
Copy link
Owner

@PzaThief
Thank you for your pull request!
I think there is no problem your souce code so, I will merge into master.

@ybkuroki ybkuroki merged commit b07a881 into ybkuroki:master Nov 12, 2023
1 check passed
@ybkuroki ybkuroki added the bug Something isn't working label Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants