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

SessionMiddleware can not work on swoole #25

Open
ChisWill opened this issue Aug 9, 2021 · 11 comments
Open

SessionMiddleware can not work on swoole #25

ChisWill opened this issue Aug 9, 2021 · 11 comments
Labels
polar status:ready for adoption Feel free to implement this issue. type:bug Bug

Comments

@ChisWill
Copy link

ChisWill commented Aug 9, 2021

Hi guys,my application run with swoole, I found I can not use SessionMiddleware because Session will be overwrite by other request.
Could we add Session into attribute of request, so we can use the way $request->getAttribute(SessionInterface::class) to get Session.
We also can remain old usage by add a new property when application running different env.

Funding

  • You can sponsor this specific effort via a Polar.sh pledge below
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@samdark samdark added the type:bug Bug label Aug 9, 2021
@samdark
Copy link
Member

samdark commented Aug 9, 2021

Do you want to keep session in the worker context and not to destroy/recreate it on each request?

@ChisWill
Copy link
Author

ChisWill commented Aug 9, 2021

No, I will create a new PsrRequest instance on each request and this is not my point.
I mean, when we use SessionInterface as a singleton and application is running on swoole,the code

if ($requestSessionId !== null && $this->session->getId() === null) {
    $this->session->setId($requestSessionId);
}

will modify session singleton and never execute setId() again when new request come.
The result is every request has same session.

@samdark
Copy link
Member

samdark commented Aug 9, 2021

Wait, but $requestSessionId is obtained from request and you have different request object every time, don't you?

$requestSessionId = $this->getSessionIdFromRequest($request);

@ChisWill
Copy link
Author

ChisWill commented Aug 9, 2021

Yes, request is different.
But, look at the if condition if ($requestSessionId !== null && $this->session->getId() === null) {
$this->session had been modified and storage in memory, so after first request the $this->session->getId() is always not null.
And at the last SessionMiddleware will commitSession(), it will modify current request session id to a same value which is $this->session->getId().

@samdark
Copy link
Member

samdark commented Aug 9, 2021

You're right. Any fix that comes in mind? I don't currently have Swoole environment set up :(

@samdark samdark added the status:ready for adoption Feel free to implement this issue. label Aug 9, 2021
@ChisWill
Copy link
Author

ChisWill commented Aug 9, 2021

As I mentioned in the question, we should use the code $request->getAttribute(SessionInterface::class) to get Session.
I think Session depends on request, it shouldn't be a singleton instance.
However, this will affect the usage of the current user, so add a property to configure?
It's up to you.

@yiiliveext
Copy link
Contributor

You're right. Any fix that comes in mind? I don't currently have Swoole environment set up :(

Make sessionId nullable.

public function setId(?string $sessionId): void
{
    $this->sessionId = $sessionId;
}

And we should add resetter in the SessionInterface::class config for those who use yiisoft/di

@ChisWill
Copy link
Author

ChisWill commented Aug 10, 2021

I think it won't work well.
In Swoole coroutine environment,suppose this situation:
Even if there is only one process and the first request come and do sleep(), the process will also accept other request.
At this time, the other request will modify the session, Until the sleeping request wakes up, his session had been changed.
So I think every request should maintain its own session copy.

@samdark
Copy link
Member

samdark commented Aug 24, 2021

  1. Added resetter config.
  2. Updated https://github.com/yiisoft/docs/blob/master/guide/en/tutorial/using-yii-with-swoole.md

@ChisWill ping us if it doesn't help please.

@samdark samdark closed this as completed Aug 24, 2021
@ChisWill
Copy link
Author

ChisWill commented Aug 25, 2021

  1. Added resetter config.
  2. Updated https://github.com/yiisoft/docs/blob/master/guide/en/tutorial/using-yii-with-swoole.md

@ChisWill ping us if it doesn't help please.

I refer to this article, but I found that in the same process session_id() always returns the same value when a new request comes.
Why not use session_create_id() or other similar method?

@samdark samdark reopened this Aug 25, 2021
@ChisWill ChisWill changed the title SessionMiddleware can not working on swoole SessionMiddleware can not work on swoole Aug 27, 2021
@alamagus
Copy link

alamagus commented Jul 28, 2022

@ChisWill
Php cli(on top of which swoole is run) has no idea about coroutines. So, at any given time there is 0 or 1 session in the process. And when you call session_* functions 'simultaneously' from different coroutines, you still operate on 1(or 0) session
src
Might be wrong tho 🤔

@polar-sh polar-sh bot added the polar label Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polar status:ready for adoption Feel free to implement this issue. type:bug Bug
Projects
None yet
Development

No branches or pull requests

4 participants