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

Yeast.yeast() is not thread safe #100

Closed
trinopoty opened this issue Jan 8, 2019 · 4 comments
Closed

Yeast.yeast() is not thread safe #100

trinopoty opened this issue Jan 8, 2019 · 4 comments
Labels

Comments

@trinopoty
Copy link

trinopoty commented Jan 8, 2019

Yeast.yeast() is not thread safe. If two thread calls it almost simultaneously, it is prone to returning same value.
I don't know how this problem has not surfaced till date but in the socket.io java server project, this is causing problems like two distinct connections getting the same id.
Also, as a side note, Yeast.yeast() uses new Date() every time which may cause problems if called frequently (memory wastage).

Can you please fix this issue and update the library.

@trinopoty
Copy link
Author

Maybe use an AtomicLong with a seed value from System.currentTimeMillis?

@darrachequesne
Copy link
Member

Hi @trinopoty, sorry for the delay on this issue.

this is causing problems like two distinct connections getting the same id

If I'm not mistaken, the string created by yeast is used in the query params, to prevent a proxy from caching the response of the server. Are you suggesting it does happen when sending several HTTP requests in a row?

Note: in the JS server, the session ID (sid in the query param) is generated by the base64id module here

@trinopoty
Copy link
Author

Yeast is using new Date().getTime() to generate a session id. On the server library, this was causing issues where two unrelated connections got the same sid because they came in at around the same time. I switched it over to an AtomicLong based implementation.
From the looks of it, this will not be too much of a problem in this library since it does not generate sid.

@darrachequesne
Copy link
Member

Oh ok, thanks for the explanation!

That being said, the yeast() method is indeed not thread safe, which might cause some problems when creating several java clients (during load tests, for example).

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

No branches or pull requests

2 participants