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

Support PHP7's read_and_close #22

Open
weierophinney opened this issue Dec 31, 2019 · 14 comments
Open

Support PHP7's read_and_close #22

weierophinney opened this issue Dec 31, 2019 · 14 comments

Comments

@weierophinney
Copy link
Member

Since PHP7 it is possible to set the option read_and_close. See: `http://php.net/manual/en/migration70.new-features.php#migration70.new-features.session-options

I use this library and want to enable this option. However it seems not to be supported yet. Would be willing to contribute. My plan is as follows.

  • Introduce this option to StandardConfig
  • Simply apply setting in SessionManager::start
  • Maybe emit a message when setting this value on PHP < 7 ??

Before I start. Does repo owner agree on the necessity and plan?


Originally posted by @roelvanduijnhoven at zendframework/zend-session#39

@weierophinney
Copy link
Member Author

This is really useful, especially if you use CQRS.


Originally posted by @sandrokeil at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

I'm not sure however who to ping at this point. Who are the core contributors to this repo?


Originally posted by @roelvanduijnhoven at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

Any response on this would be welcome!


Originally posted by @roelvanduijnhoven at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

@weierophinney can you take a (quick) look at this?


Originally posted by @kokx at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

I think your plan only deals with the first part of the story. Using "read_and_close" is the equivalent of calling session_start() and session_write_close() consecutively. Which means that calling SessionManager::start() now should behave as if SessionManager::writeClose() has been called directly after that if the option has been set. I cannot asses whether this has any implications - the session storage objects do have the immutable flag which would allow inhibiting write access.

However, the interesting part would be outside of this module: How would a software detect that it does not need to write back to the session? And why would this decision be transmitted as a configuration flag into the StandardConfig object - it is somehow dynamic in nature, being different for every single request depending at least on the route, but maybe also on the data.


Originally posted by @SvenRtbg at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

How would a software detect that it does not need to write back to the session?

I guess that's only done by convention. GET requests only read, other requests like POST/PATCH read/write.


Originally posted by @sandrokeil at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

Some clarification: I thought I needed this piece of functionality. But turns out it doesn't solve what I was hoping for. Thus I have no real use case anymore! Is there anybody that does need this? Otherwise I could simply close.


Originally posted by @roelvanduijnhoven at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

I think at a minimum, SessionManager->start should accept an option to enable read_and_close.

I don't think the module itself can make the decision on when to use read_and_close and when not to use it. Ideally people would use GET and POST/PATCH correctly and GET would never have any side affects, but in reality it probably will.

Its best to leave that determination up to the application.


Originally posted by @datasage at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

@roelvanduijnhoven a simple use case where the the read_and_close behaviour is extremely useful is a situation with multiple simultaneous AJAX requests, each starting a session - if these AJAX requests don't need to write to the session (but still need to read from it), each of those AJAX requests will be blocking the session file (in my case, for 2s), effectively rendering them synchronous (and furthermore, blocking the workers spool)


Originally posted by @eithed at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

@eithed I'd consider your suggestion a "case where you'd use it", but it is barely a use case in the light of the original request and it's plan.

Detail a code situation where you'd have an application, which may allow for some requests to immediately close and release the session, and also consider the way you'd access the session data.

Note: The last time I looked, the native PHP session itself was started implicitly by explicitly using a session container object. Prior to that, the session manager is configured by the general application settings regarding sessions, i.e. cookie name etc.

This would make the original plan unfeasible: You cannot configure the session manager to immediately close the session because this info usually is only ever accessible after the routing has determined the controller and action. At this stage in a ZF application you usually do not configure the session manager anymore. Also, your controller will likely only get some models injected, which may internally use some instances of session containers, which will implicitly start the PHP session.

It might be possible to instantiate these session containers in a "readonly" mode. However, they would still be used in a generic model use case, i.e. the model needs to know in advance if the controller is about to call methods which would write to the session or not.

All in all, if you decide to use PHP sessions, you should know that this comes with a locking feature which will serialize parallel AJAX execution. The solution for this problem likely is to not use PHP sessions, but to use a storage that can be queried without locking. However, this immediately calls for some more code to prevent unintentionally overwriting the stored data. Event sourcing with append-only writing may be a solution - but this clearly is beyond the scope of this zend-session library.

The other way to solve the problem is to call SessionManager::writeClose() from your code immediately after you know that nothing will be written to the session anymore. But this is already implemented.

You'd have a use case if you can counter my arguments and detail a scenario which would allow to know that the session that still isn't started can be started with the read_and_close option, and which also is in line with the current solution that this library provides and which hasn't been implemented yet.


Originally posted by @SvenRtbg at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

@SvenRtbg I think my use case already presents a sensible situation and the effects - given three simultaneous requests, I've observed request times of 90ms, 2s and 4s, where each of the requests repeated singularly would end up finishing within 90ms. The problem was session locking which I've not thought of before. The requests in question were simple JSON returning requests, where session was required, but writing to it wasn't needed. (As a sidenote I failed to find a setting which would explain why was the session file locked for 2s in each of the requests, even though each of the scripts would finish in less than 2s)

I don't understand what routing has to do with the issue - yes, some models may use sessions, or they may not - it is up to the application (and the creators) to determine if they want a writable session, or not - just like it was brought up by @datasage : usually GET requests will be read-only, while other than GET will require writing to the session (not always, but there are multiple ways to pass that information - via headers, request params, even url matching)

I'm under the impression that session_start(['read_and_close' => true]) initializes session in read-only mode (I might be under wrong impression) - but this is exactly what's required: to read the data from session without setting the lock on it. Sure - I can do session_start(['read_and_close' => true]) and then proceed using the Zend Session wrapper (an icky solution); I can do as you've mentioned with immediately calling SessionManager::writeClose() - while there's a difference (opening session in read-only to opening session in write and closing it within, hopefully, same cycle) the end result is indeed the same.

I'd like to see this option, as it makes sense, as the support for it exists outside the wrapper. Yes, there are ways to emulate it, so there's no harm done if it's not there.


Originally posted by @eithed at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

Routing is involved because usually your application does not only provide URLs that read from the session. So based on which URL has been requested, some of them write to the session and others don't. This means that the application can only know if it is possible to read_and_close after the routing has been done.

Also I'd oppose the idea that GET requests will not write to the session. This method should behave idempotent, but this is only related to the server response, not necessarily to the server state itself. But beside that discussion, determining whether a request uses GET or another HTTP method still is the task of routing, and the result is not available before that.

In other news, your session lock timing suspiciously looks like something waits for a timeout. Which session save handler are you using? I've only encountered this "useless waiting until the next full second" when using something different than files.


Originally posted by @SvenRtbg at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

@eithed Looking at the PHP source code, it is clear that session_start(['read_and_close' => true]) still acquires a lock on the shared storage resource, but will immediately release it without writing the session data back. This is an improvement compared to session_write_close() because it omits the write, but it still locks the data for a short time, so I doubt your timeout problem would go away, it might only be reduced. You'd have a much smaller time range in which a request holds the lock, so a concurrent request is more likely to not see that lock when starting the session. But if the resource is locked, that request still has to wait until it's release, and it seems like this is only done every two seconds for your particular session save handler.


Originally posted by @SvenRtbg at zendframework/zend-session#39 (comment)

@weierophinney
Copy link
Member Author

Let's imagine a scenario, where presence of header: "Session: read-only" sets the session in aforementioned state. If routing was the only thing that can (or should) access headers, then I'd say that I can see and agree with the point you're making. I however think otherwise (are we getting in to personal preference territory?)

I'm using memcache, and I actually found where the 2s come from - it's the timeout of the memcache connection. Still it doesn't explain why memcache is not notified about the release of the session, but manually closing the session (either with read_and_close, or session_write_close) helps. It might be that session lock release is transmitted after memcache has been unloaded, but I lack capabilities to find that out (this would however explain things - initial request: memcache connection is unloaded, session is closed which can't notify the memcache, next request: hangs on session lock without being able to access the session, until the initial memcache connection times out)


Originally posted by @eithed at zendframework/zend-session#39 (comment)

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

No branches or pull requests

1 participant