FAQ
Hi internals,

After feedback on the original proposal, I've updated the RFC about
changing lazy_write, read_only and I'm re-posting it. The first
discussion thread got somewhat spammy, so I thought a new one should
be better.

https://wiki.php.net/rfc/session-read_only-lazy_write

Waiting for your comments on here. :)

Regards,
Andrey Andreev.

Search Discussions

  • Bill Salak at Mar 24, 2014 at 5:39 pm

    After feedback on the original proposal, I've updated the RFC about changing lazy_write, read_only and I'm re-posting it. The first discussion thread got somewhat spammy, so I thought a new one should be better.

    https://wiki.php.net/rfc/session-read_only-lazy_write

    Waiting for your comments on here. :)

    Regards,
    Andrey Andreev.
    Hi Andrey,
    In your proposed example problematic use case your code shows that the following happens:

      - Request 1, opens session, populates _SESSION superglobal, closes session (no writeback)
      - Request 2, opens sessions, unsets $_SESSION['logged_in'], writes session
      - Request 1, checks value of $_SESSION['logged_in'] and does not recognize that the value of $_SESSION['logged_in'] has changed by Request 2

    The net result for Request 1 is exactly the same as what we have today except that today the path to this same result is that Request 2 can't read or modify the data until Request 1 finishes. Because this is not a new or unexpected result I don't see how this is "downright dangerous" unless the user misunderstood what the new functionality does and designed solutions around this misunderstanding that exposed problems in their new code. I think this is actually the core of your RFP on this point which I think just boils down to - is read_only the best name for this flag ? I personally think it's a descriptive and self-evident name for the option because the session in Request 1 *is* read only (can't be written to) and that what you think of as a read only is actually better called a "read lock". Ultimately I don't care what it's called so if that's all this is about then I have no more interest in this part of the discussion and am happy to let those who care determine the name of the option.

    On your point about some future implementation of a write blocking, read-shared, session lock - it's an interesting idea.

    On your point about, "Maybe, if session_start() didn't accept mode parameters, that would've been fine. However, session_start() also accepts all session.* INIs + 'lazy_write' and all of those are modes of operation and not additional actions per se. So that makes it not only strange, but also inconsistent", you've lost me -I don't see a problem. If I call session_start and I can pass in a bunch of options about how the session will act in this call stack that seems like the best and most pragmatic solution. The distinction between modes of operation and additional actions seem like a semantic nitpick that end-users wouldn't intuitively understand. In other words, it seems counter-intuitive to work some other way and wouldn't produce more easily read/written code to have it different.

    Best,
    Bill Salak
  • Andrey Andreev at Mar 24, 2014 at 7:49 pm
    Hi,
    On Mon, Mar 24, 2014 at 7:39 PM, Bill Salak wrote:
    Hi Andrey,
    In your proposed example problematic use case your code shows that the following happens:

    - Request 1, opens session, populates _SESSION superglobal, closes session (no writeback)
    - Request 2, opens sessions, unsets $_SESSION['logged_in'], writes session
    - Request 1, checks value of $_SESSION['logged_in'] and does not recognize that the value of $_SESSION['logged_in'] has changed by Request 2

    The net result for Request 1 is exactly the same as what we have today except that today the path to this same result is that Request 2 can't read or modify the data until Request 1 finishes. Because this is not a new or unexpected result I don't see how this is "downright dangerous" unless the user misunderstood what the new functionality does and designed solutions around this misunderstanding that exposed problems in their new code. I think this is actually the core of your RFP on this point which I think just boils down to - is read_only the best name for this flag ? I personally think it's a descriptive and self-evident name for the option because the session in Request 1 *is* read only (can't be written to) and that what you think of as a read only is actually better called a "read lock". Ultimately I don't care what it's called so if that's all this is about then I have no more interest in this part of the discussion and am happy to let those who care determine the name of the option.
    The issue is exactly that it would be easy to not understand what
    'read_only' does. Nowhere have I said that the functionality itself is
    dangerous or that it is not useful, exactly the opposite - I love the
    idea, it's pretty neat. My problem is with how it is named, and that's
    what the RFC talks about. :)
    Unless I'm mistaken, you already mentioned in another thread that it
    is you who suggested the 'read_only' name, so I guess it's natural
    that you don't see a problem with it and that it seems descriptive and
    self-evident to you. Somebody else already applied the same logic - it
    only reads and it doesn't allow a write. However, as described in the
    RFC, the _close immediately_ part is very unclear.
    I don't want to get into lengthy arguments with you or somebody else
    over what "read-only" means. I'm not just talking about my
    understanding of it here ... the term already has a meaning that is
    recognized everywhere (google it if you don't believe me) and that's
    why I'm so strongly against keeping it as it is.
    On your point about some future implementation of a write blocking, read-shared, session lock - it's an interesting idea.
    Glad you like it, but as I already told you previously - it's just for
    reference, I'm not interested in adding that feature right now.
    On your point about, "Maybe, if session_start() didn't accept mode parameters, that would've been fine. However, session_start() also accepts all session.* INIs + 'lazy_write' and all of those are modes of operation and not additional actions per se. So that makes it not only strange, but also inconsistent", you've lost me -I don't see a problem. If I call session_start and I can pass in a bunch of options about how the session will act in this call stack that seems like the best and most pragmatic solution. The distinction between modes of operation and additional actions seem like a semantic nitpick that end-users wouldn't intuitively understand. In other words, it seems counter-intuitive to work some other way and wouldn't produce more easily read/written code to have it different.
    Well, I certainly can't understand why you think that a separate
    function would be counter-intuitive or that it won't produce
    easily-read code. With what we currently have, chances are that the
    following line would be seen quite often:

         session_start($options);

    What do you understand from that line (regardless of whether
    'read_only' is in $options or not)? I see "start a session with some
    options". This is again where the closing part is lost, nothing
    implies that anything but "start a session" would be performed, as an
    action. While on the other hand:

         session_start_close($options);

    I'm quite certain that everybody would have a better understanding of
    what this line does, simply because it's explicit.
    Yes, it is nitpicky and it's nothing but semantics, but semantics are
    important. :)

    Cheers,
    Andrey.
  • Bill Salak at Mar 25, 2014 at 12:22 am
    Andrey,
    The issue is exactly that it would be easy to not understand what 'read_only' does. Nowhere have I said that the functionality itself is dangerous
    or that it is not useful, exactly the opposite - I love the idea, it's pretty neat. My problem is with how it is named, and that's what the RFC talks
    about. :) Unless I'm mistaken, you already mentioned in another thread that it is you who suggested the 'read_only' name, so I guess it's natural
    that you don't see a problem with it and that it seems descriptive and self-evident to you.
    I only suggested the functionality, not the name of the option. Interestingly enough I originally suggested it as a new function just like you suggest
    but Yasuo asked me if I thought it would be better as an option. After mocking up some code using both I found the option passed to session_start
    to give me more flexibility in my solution design and I found it more intuitive so I agreed it was better as an option passed into session_start and
    not a function.

    ...
    On your point about, "Maybe, if session_start() didn't accept mode parameters, that would've been fine. However, session_start() also accepts
    all session.* INIs + 'lazy_write' and all of those are modes of operation and not additional actions per se. So that makes it not only strange, but
    also inconsistent", you've lost me -I don't see a problem. If I call session_start and I can pass in a bunch of options about how the session will
    act in this call stack that seems like the best and most pragmatic solution. The distinction between modes of operation and additional actions
    seem like a semantic nitpick that end-users wouldn't intuitively understand. In other words, it seems counter-intuitive to work some other way
    and wouldn't produce more easily read/written code to have it different.
    Well, I certainly can't understand why you think that a separate function would be counter-intuitive or that it won't produce easily-read code. With
    what we currently have, chances are that the following line would be seen quite often:

    session_start($options);

    What do you understand from that line (regardless of whether 'read_only' is in $options or not)? I see "start a session with some options". This is
    again where the closing part is lost, nothing implies that anything but "start a session" would be performed, as an action. While on the other hand:

    session_start_close($options);

    I'm quite certain that everybody would have a better understanding of what this line does, simply because it's explicit.
    Yes, it is nitpicky and it's nothing but semantics, but semantics are important. :)
    To answer your question, as a single line yes, it read more clearly in regards to that 1 characteristic of the session, but...
    when I wrote my mockups I ran through a bunch of these scenarios and found myself wrapping session_start and what you call session_start_close
    with a php end user function so I could consolidate my calls to start a session into a common interface that accepted an argument to tell the function
    which php native function to call to start the session. Maybe that's just me but I think this will in fact become common in codebases which liberally use
    both types of sessions starts. I went down this path pretty quickly when running through use cases with my existing codebases and it felt unnecessarily
    restrictive to me considering it was solved by the passing as an option to session_start solution Yasuo had suggested at that time.

    The separate function approach also doesn't account for adding more things like this in the future or to chain those options together in the same session
    without creating a new function for every permutation possible for any new options we add unless this acts as a one-off and all future options like this
    are added as flags passed into via the options array. In other words, as a rule "make it a function not an option" doesn't scale if we add more stuff like this
    in the future.

    The separate function approach also doesn't cleanly (cleanly is my subjective opinion) support the option having some value outside of TRUE,
    i.e. session_start(['read_only'=>FOO]) might be a viable option at some point.

    Best,
    Bill Salak
  • Andrey Andreev at Mar 25, 2014 at 10:02 am
    Hi,
    Well, I certainly can't understand why you think that a separate function would be counter-intuitive or that it won't produce easily-read code. With
    what we currently have, chances are that the following line would be seen quite often:

    session_start($options);

    What do you understand from that line (regardless of whether 'read_only' is in $options or not)? I see "start a session with some options". This is
    again where the closing part is lost, nothing implies that anything but "start a session" would be performed, as an action. While on the other hand:

    session_start_close($options);

    I'm quite certain that everybody would have a better understanding of what this line does, simply because it's explicit.
    Yes, it is nitpicky and it's nothing but semantics, but semantics are important. :)
    To answer your question, as a single line yes, it read more clearly in regards to that 1 characteristic of the session, but...
    when I wrote my mockups I ran through a bunch of these scenarios and found myself wrapping session_start and what you call session_start_close
    with a php end user function so I could consolidate my calls to start a session into a common interface that accepted an argument to tell the function
    which php native function to call to start the session. Maybe that's just me but I think this will in fact become common in codebases which liberally use
    both types of sessions starts. I went down this path pretty quickly when running through use cases with my existing codebases and it felt unnecessarily
    restrictive to me considering it was solved by the passing as an option to session_start solution Yasuo had suggested at that time.
    If you don't have to wrap the function call, you'll have to wrap the
    option somehow, you can't escape from that. Thankfully it's something
    that's only written once within a single application, hence why it
    should be more explicit and easily recognizable.
    Otherwise, everybody is entitled an opinion and their own preferences,
    so we'll never agree on which one feels better in general.
    The separate function approach also doesn't account for adding more things like this in the future or to chain those options together in the same session
    without creating a new function for every permutation possible for any new options we add unless this acts as a one-off and all future options like this
    are added as flags passed into via the options array. In other words, as a rule "make it a function not an option" doesn't scale if we add more stuff like this
    in the future.

    The separate function approach also doesn't cleanly (cleanly is my subjective opinion) support the option having some value outside of TRUE,
    i.e. session_start(['read_only'=>FOO]) might be a viable option at some point.
    I don't see the potental for neither another similar option
    (representing an action instead of mode) or another possible value.
    Pretty much every session-related action has its own function now, I
    consider this one to be an edge case.

    Cheers,
    Andrey.
  • Andrey Andreev at Mar 25, 2014 at 12:55 pm
    Hi,

    In order to avoid further arguments about whether a separate function
    for read-and-close is better or not, I've added an alternative
    proposal - to rename the option to 'read_close' or 'read_and_close'.
    After all, the most important thing is that it's not 'read_only'.

    Cheers,
    Andrey.
  • Julien Pauli at Mar 26, 2014 at 10:30 am

    On Tue, Mar 25, 2014 at 1:55 PM, Andrey Andreev wrote:

    Hi,

    In order to avoid further arguments about whether a separate function
    for read-and-close is better or not, I've added an alternative
    proposal - to rename the option to 'read_close' or 'read_and_close'.
    After all, the most important thing is that it's not 'read_only'.

    I agree "read_and_close" is much better discribing what it really does , so
    I prefer it.

    For non BC changes etc.. , please, consider that you'll have a big time for
    rethinking the whole session module for PHP6 if you want to (and I think
    I'll be part of deep discussions here)
    So don't bother too much in searching solutions for introducting new
    concepts in PHP5.X session module while keeping BC. Keep all those for PHP6.

    We are near 5.6 freeze, not that I dont want new shinny features, but what
    I want for 5.6 is something both consistent and voted, should it be "just a
    tiny feature".
    Work and thoughts are not lost anyway.

    Julien.P
  • Andrey Andreev at Mar 26, 2014 at 11:15 am
    Hi,
    On Wed, Mar 26, 2014 at 12:29 PM, Julien Pauli wrote:
    For non BC changes etc.. , please, consider that you'll have a big time for
    rethinking the whole session module for PHP6 if you want to (and I think
    I'll be part of deep discussions here)
    So don't bother too much in searching solutions for introducting new
    concepts in PHP5.X session module while keeping BC. Keep all those for PHP6.
    Of course, I've got more ideas for PHP6, but what I'm targeting here
    are already voted features that could've been designed in a better
    way.

    Btw, another argument in merging updateTimestamp() into write() is
    that userland implementations of it up until now have only been
    possible by altering write(), so at least some users should be more
    familiar with that approach. For example:

    class MySessionHandler extends SessionHandler {

         public $fingerprint;

         public function read($session_id)
         {
             $session_data = parent::read($session_id);
             $this->fingerprint = md5($session_data);
             return $session_data;
         }

         public function write($session_id, $session_data)
         {
             if ($this->fingerprint === md5($session_data)
             {
                 return touch(ini_get('session.save_path').'/sess_'.$session_id);
             }

             return parent::write($session_id, $session_data);
         }

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Mar 28, 2014 at 7:07 am
    Hi Andrey,
    On Wed, Mar 26, 2014 at 8:15 PM, Andrey Andreev wrote:

    Btw, another argument in merging updateTimestamp() into write() is
    that userland implementations of it up until now have only been
    possible by altering write(), so at least some users should be more
    familiar with that approach.
    Component only does its jobs with good implementation.
    Session module consists of session manager, session save handler and session
    serializer.

    Session manager should manage how it works.
    Session save handler should save/retrieve session data only.
    Session serializer should serialize/unserialize data only.

    It breaks this design with your suggestion.
    So letting save handler do the manager's job is not good. IMO.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Andrey Andreev at Mar 28, 2014 at 9:09 am
    Hi,
    On Fri, Mar 28, 2014 at 9:07 AM, Yasuo Ohgaki wrote:
    Hi Andrey,
    On Wed, Mar 26, 2014 at 8:15 PM, Andrey Andreev wrote:

    Btw, another argument in merging updateTimestamp() into write() is
    that userland implementations of it up until now have only been
    possible by altering write(), so at least some users should be more
    familiar with that approach.

    Component only does its jobs with good implementation.
    Session module consists of session manager, session save handler and session
    serializer.

    Session manager should manage how it works.
    Session save handler should save/retrieve session data only.
    Session serializer should serialize/unserialize data only.

    It breaks this design with your suggestion.
    So letting save handler do the manager's job is not good. IMO.
    I don't understand how that's breaking any kind of design. Could you elaborate?

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Mar 28, 2014 at 9:17 am
    Hi Andrey,
    On Fri, Mar 28, 2014 at 6:09 PM, Andrey Andreev wrote:

    Hi,
    On Fri, Mar 28, 2014 at 9:07 AM, Yasuo Ohgaki wrote:
    Hi Andrey,
    On Wed, Mar 26, 2014 at 8:15 PM, Andrey Andreev wrote:

    Btw, another argument in merging updateTimestamp() into write() is
    that userland implementations of it up until now have only been
    possible by altering write(), so at least some users should be more
    familiar with that approach.

    Component only does its jobs with good implementation.
    Session module consists of session manager, session save handler and session
    serializer.

    Session manager should manage how it works.
    Session save handler should save/retrieve session data only.
    Session serializer should serialize/unserialize data only.

    It breaks this design with your suggestion.
    So letting save handler do the manager's job is not good. IMO.
    I don't understand how that's breaking any kind of design. Could you
    elaborate?
    Manager should manage how session behaves, not save handlers.
    It's basic principle of modular design.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Andrey Andreev at Mar 28, 2014 at 9:46 am
    Hi,
    Btw, another argument in merging updateTimestamp() into write() is
    that userland implementations of it up until now have only been
    possible by altering write(), so at least some users should be more
    familiar with that approach.

    Component only does its jobs with good implementation.
    Session module consists of session manager, session save handler and
    session
    serializer.

    Session manager should manage how it works.
    Session save handler should save/retrieve session data only.
    Session serializer should serialize/unserialize data only.

    It breaks this design with your suggestion.
    So letting save handler do the manager's job is not good. IMO.
    I don't understand how that's breaking any kind of design. Could you
    elaborate?

    Manager should manage how session behaves, not save handlers.
    It's basic principle of modular design.
    I still don't get it ... the session manager has to call either
    write() or updateTimestamp() and both of these are part of the session
    handler. Merging them into one solves the API design and BC issues, I
    don't see how it breaks any principle. They can still be split to two
    methods in PHP6, but for the time being, using write() for both
    purposes IMO solves way more problems than sticking to this design
    principle you're talking about.

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Mar 28, 2014 at 10:32 am
    Hi Andrey,
    On Fri, Mar 28, 2014 at 6:46 PM, Andrey Andreev wrote:

    Manager should manage how session behaves, not save handlers.
    It's basic principle of modular design.
    I still don't get it ... the session manager has to call either
    write() or updateTimestamp() and both of these are part of the session
    handler. Merging them into one solves the API design and BC issues, I
    don't see how it breaks any principle. They can still be split to two
    methods in PHP6, but for the time being, using write() for both
    purposes IMO solves way more problems than sticking to this design
    principle you're talking about.

    Without API, manager cannot manage how it behaves. In general, submodule
    should
    avoid manager state dependency in general. It should have dedicated API for
    each
    distinct task rather than leaving it to managed by submodule. In addition,
    manager
    cannot know if save handler supports API or not. If there is API, I can
    display save
    handler capability in phpinfo() page, for example.

    If manager expects sub module to behave in some way, it should have
    explicit API
    for each feature. Otherwise, sub module implementation may differ module by
    module.
    Defined set of feature is better to have explicit API with modular design.
    It's not mandatory,
    but the best practice. I don't see reason not to follow the practice here.

    Besides modular design, if write() and updateTimestamp() are merged, flag
    parameter
    for write() should be added. It breaks compatibility with current save
    handlers. I don't
    want BC that could be avoided also.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Andrey Andreev at Mar 28, 2014 at 10:50 am
    Hi,
    I still don't get it ... the session manager has to call either
    write() or updateTimestamp() and both of these are part of the session
    handler. Merging them into one solves the API design and BC issues, I
    don't see how it breaks any principle. They can still be split to two
    methods in PHP6, but for the time being, using write() for both
    purposes IMO solves way more problems than sticking to this design
    principle you're talking about.

    Without API, manager cannot manage how it behaves. In general, submodule
    should
    avoid manager state dependency in general. It should have dedicated API for
    each
    distinct task rather than leaving it to managed by submodule. In addition,
    manager
    cannot know if save handler supports API or not. If there is API, I can
    display save
    handler capability in phpinfo() page, for example.

    If manager expects sub module to behave in some way, it should have explicit
    API
    for each feature. Otherwise, sub module implementation may differ module by
    module.
    Defined set of feature is better to have explicit API with modular design.
    It's not mandatory,
    but the best practice. I don't see reason not to follow the practice here.
    I agree in general, but you just gave a good reason not to follow that
    practice - you can't know if the submodule supports it. Plus, both are
    write operations with one of them just writing more data. I'm all for
    best practices, but in this case there's a lot of sense not to do
    everything by the book.
    Besides modular design, if write() and updateTimestamp() are merged, flag
    parameter
    for write() should be added. It breaks compatibility with current save
    handlers. I don't
    want BC that could be avoided also.
    No it shouldn't, the decision whether to write or just update the
    timestamp is based on an internal flag, or on $session_data. No
    additional parameters are required.

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Mar 28, 2014 at 10:03 pm
    Hi Andrey,
    On Fri, Mar 28, 2014 at 7:50 PM, Andrey Andreev wrote:

    I still don't get it ... the session manager has to call either
    write() or updateTimestamp() and both of these are part of the session
    handler. Merging them into one solves the API design and BC issues, I
    don't see how it breaks any principle. They can still be split to two
    methods in PHP6, but for the time being, using write() for both
    purposes IMO solves way more problems than sticking to this design
    principle you're talking about.

    Without API, manager cannot manage how it behaves. In general, submodule
    should
    avoid manager state dependency in general. It should have dedicated API for
    each
    distinct task rather than leaving it to managed by submodule. In addition,
    manager
    cannot know if save handler supports API or not. If there is API, I can
    display save
    handler capability in phpinfo() page, for example.

    If manager expects sub module to behave in some way, it should have explicit
    API
    for each feature. Otherwise, sub module implementation may differ module by
    module.
    Defined set of feature is better to have explicit API with modular design.
    It's not mandatory,
    but the best practice. I don't see reason not to follow the practice
    here.

    I agree in general, but you just gave a good reason not to follow that
    practice - you can't know if the submodule supports it. Plus, both are
    write operations with one of them just writing more data. I'm all for
    best practices, but in this case there's a lot of sense not to do
    everything by the book.
    If there is API, sub module capability can be detected.

    Writing data and updating time stamp for GC is distinct feature.
    I don't see good reason not to have API for it.

    Besides modular design, if write() and updateTimestamp() are merged, flag
    parameter
    for write() should be added. It breaks compatibility with current save
    handlers. I don't
    want BC that could be avoided also.
    No it shouldn't, the decision whether to write or just update the
    timestamp is based on an internal flag, or on $session_data. No
    additional parameters are required.

    I removed PS(id) dependency from s_read() with new patch as planned. Why
    should I introduce new dependency to s_write(), i.e. sub module, that
    breaks
    design? It does not make sense.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Yasuo Ohgaki at Mar 28, 2014 at 10:28 pm
    Hi all,
    On Sat, Mar 29, 2014 at 7:02 AM, Yasuo Ohgaki wrote:

    I removed PS(id) dependency from s_read() with new patch as planned.

    Additional note for this.
    Dependency was needed to keep compatibility. Since this change is done in
    release version for use_strict_mode, BC has much higher priority than clean
    code.
    Since 5.6 is new release, new API, i.e. PS_VALIDATE_SID, is introduced and
    code
    is cleaned up as planned.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Andrey Andreev at Mar 28, 2014 at 11:41 pm
    Hi,
    Without API, manager cannot manage how it behaves. In general, submodule
    should
    avoid manager state dependency in general. It should have dedicated API
    for
    each
    distinct task rather than leaving it to managed by submodule. In
    addition,
    manager
    cannot know if save handler supports API or not. If there is API, I can
    display save
    handler capability in phpinfo() page, for example.

    If manager expects sub module to behave in some way, it should have
    explicit
    API
    for each feature. Otherwise, sub module implementation may differ module
    by
    module.
    Defined set of feature is better to have explicit API with modular
    design.
    It's not mandatory,
    but the best practice. I don't see reason not to follow the practice
    here.
    I agree in general, but you just gave a good reason not to follow that
    practice - you can't know if the submodule supports it. Plus, both are
    write operations with one of them just writing more data. I'm all for
    best practices, but in this case there's a lot of sense not to do
    everything by the book.

    If there is API, sub module capability can be detected.
    You yourself told me that there's no way for the SessionHandler class
    to know if the storage module supports PS_UPDATE_TIMESTAMP_FUNC() or
    not, and that because of that we can't have
    SessionHandler::updateTimestamp().
    Writing data and updating time stamp for GC is distinct feature.
    For file-based sessions - it is, but for i.e. a database, there's not
    much difference. write() does update the timestamp, I see no reason
    why it shouldn't be able to update just the timestamp.
    I don't see good reason not to have API for it.
      - Not being able to call parent::updateTimestamp() is a good reason.
      - Having the completely unnecessary SessionUpdateTimestampInterface
    is a good reason.
      - Breaking the design of current userland implementations is a good reason.
      - Not being able to have lazy_write (a performance improvement, I
    remind you) by default is a good reason.
    Besides modular design, if write() and updateTimestamp() are merged,
    flag
    parameter
    for write() should be added. It breaks compatibility with current save
    handlers. I don't
    want BC that could be avoided also.
    No it shouldn't, the decision whether to write or just update the
    timestamp is based on an internal flag, or on $session_data. No
    additional parameters are required.

    I removed PS(id) dependency from s_read() with new patch as planned. Why
    should I introduce new dependency to s_write(), i.e. sub module, that breaks
    design? It does not make sense.
    Convenience and consistency in the userland APIs is more important
    than "breaking" internal design. I understand you want to stick to
    best practices, but those practices are not a religion and sometimes
    you need to break them. If you ask me, a lot of things in ext/session
    are broken by design ... creating files from inside read() is one
    example, all methods exposed to userland to be in an interface is
    another. I agree, it's not perfect, but that's just what happens when
    we introduce new features into something that was never designed with
    possible changes in the future in mind. For PHP6, we'll have the
    freedom to redesign the whole thing completely, but for the time being
    - userland code is more important than not having some dependancy in
    write().

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Mar 29, 2014 at 1:10 am
    Hi Andrey,
    On Sat, Mar 29, 2014 at 8:41 AM, Andrey Andreev wrote:

    You yourself told me that there's no way for the SessionHandler class
    to know if the storage module supports PS_UPDATE_TIMESTAMP_FUNC() or
    not, and that because of that we can't have
    SessionHandler::updateTimestamp().
    It's only applicable for object based user save handler.
    Since user is implemented their own handler, this limitation should not
    matter.

    Writing data and updating time stamp for GC is distinct feature.
    For file-based sessions - it is, but for i.e. a database, there's not
    much difference. write() does update the timestamp, I see no reason
    why it shouldn't be able to update just the timestamp.
    How to update time stamp is up to save handler and its storage.
    Write and updating time stamp is different operation.
    Memcache does not need it if data is read. RDBMS may have separate
    table to keep track time stamp. For PostgreSQL, updating one field is
    faster than updating whole record.


    I don't see good reason not to have API for it.
    - Not being able to call parent::updateTimestamp() is a good reason.
    - Having the completely unnecessary SessionUpdateTimestampInterface
    is a good reason.
    - Breaking the design of current userland implementations is a good
    reason.
    - Not being able to have lazy_write (a performance improvement, I
    remind you) by default is a good reason.
    I think current design of object based save handler is better to be
    redesigned.

    Current object based save handler registers "previous" save handler as its
    base.
    However, it's mostly useless without calling parent open() function. i.e.
    Other
    calls simply fails when it does not.

    When open() is called, some resource, e.g. file handle, db connection, etc,
    is
    opened. These resources cannot be accessed from user land, since it's not
    "PHP resource", but raw resource. Thus it's only useful for file based
    storage.
    If user is using their own file based storage for some reason, they are
    better
    to implement their own handler fully.

    I would like to remove and clean up this in future release.

    Besides modular design, if write() and updateTimestamp() are merged,
    flag
    parameter
    for write() should be added. It breaks compatibility with current save
    handlers. I don't
    want BC that could be avoided also.
    No it shouldn't, the decision whether to write or just update the
    timestamp is based on an internal flag, or on $session_data. No
    additional parameters are required.

    I removed PS(id) dependency from s_read() with new patch as planned. Why
    should I introduce new dependency to s_write(), i.e. sub module, that breaks
    design? It does not make sense.
    Convenience and consistency in the userland APIs is more important
    than "breaking" internal design. I understand you want to stick to
    best practices, but those practices are not a religion and sometimes
    you need to break them. If you ask me, a lot of things in ext/session
    are broken by design ... creating files from inside read() is one
    example, all methods exposed to userland to be in an interface is
    another. I agree, it's not perfect, but that's just what happens when
    we introduce new features into something that was never designed with
    possible changes in the future in mind. For PHP6, we'll have the
    freedom to redesign the whole thing completely, but for the time being
    - userland code is more important than not having some dependancy in
    write().
    It's better to stick to cleaner code. Clean code is worth to have.
    As I mentioned in previous, current object based save handler design has
    very
    limited usage and base class part would be better to be removed in the
    future.
    (It's not documented also :)

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Andrey Andreev at Mar 29, 2014 at 1:47 am

    On Sat, Mar 29, 2014 at 3:09 AM, Yasuo Ohgaki wrote:
    Hi Andrey,
    On Sat, Mar 29, 2014 at 8:41 AM, Andrey Andreev wrote:

    You yourself told me that there's no way for the SessionHandler class
    to know if the storage module supports PS_UPDATE_TIMESTAMP_FUNC() or
    not, and that because of that we can't have
    SessionHandler::updateTimestamp().

    It's only applicable for object based user save handler.
    Since user is implemented their own handler, this limitation should not
    matter.
    You're saying this like nobody uses the SessionHandler class. You
    don't know that, it's just an assumption of yours, and based on it
    you're willing to have a limited instead of a complete feature, while
    there are no real technical limitations. This is surprising,
    considering that you cite "lack of API" for other features that are
    IMO not as useful.
    Writing data and updating time stamp for GC is distinct feature.
    For file-based sessions - it is, but for i.e. a database, there's not
    much difference. write() does update the timestamp, I see no reason
    why it shouldn't be able to update just the timestamp.

    How to update time stamp is up to save handler and its storage.
    Write and updating time stamp is different operation.
    Memcache does not need it if data is read. RDBMS may have separate
    table to keep track time stamp. For PostgreSQL, updating one field is
    faster than updating whole record.
    I'm not saying it isn't faster to update just one field, I'm saying
    it's still an UPDATE clause in both cases.
    I don't see good reason not to have API for it.
    - Not being able to call parent::updateTimestamp() is a good reason.
    - Having the completely unnecessary SessionUpdateTimestampInterface
    is a good reason.
    - Breaking the design of current userland implementations is a good
    reason.
    - Not being able to have lazy_write (a performance improvement, I
    remind you) by default is a good reason.

    I think current design of object based save handler is better to be
    redesigned.

    Current object based save handler registers "previous" save handler as its
    base.
    However, it's mostly useless without calling parent open() function. i.e.
    Other
    calls simply fails when it does not.

    When open() is called, some resource, e.g. file handle, db connection, etc,
    is
    opened. These resources cannot be accessed from user land, since it's not
    "PHP resource", but raw resource. Thus it's only useful for file based
    storage.
    If user is using their own file based storage for some reason, they are
    better
    to implement their own handler fully.

    I would like to remove and clean up this in future release.
    You don't know how it's used. A user may just prepend the session_id
    before calling parent::, or inject logging into the logic, or whatever
    - people do all kinds of crazy stuff.
    Besides modular design, if write() and updateTimestamp() are merged,
    flag
    parameter
    for write() should be added. It breaks compatibility with current
    save
    handlers. I don't
    want BC that could be avoided also.
    No it shouldn't, the decision whether to write or just update the
    timestamp is based on an internal flag, or on $session_data. No
    additional parameters are required.

    I removed PS(id) dependency from s_read() with new patch as planned. Why
    should I introduce new dependency to s_write(), i.e. sub module, that
    breaks
    design? It does not make sense.
    Convenience and consistency in the userland APIs is more important
    than "breaking" internal design. I understand you want to stick to
    best practices, but those practices are not a religion and sometimes
    you need to break them. If you ask me, a lot of things in ext/session
    are broken by design ... creating files from inside read() is one
    example, all methods exposed to userland to be in an interface is
    another. I agree, it's not perfect, but that's just what happens when
    we introduce new features into something that was never designed with
    possible changes in the future in mind. For PHP6, we'll have the
    freedom to redesign the whole thing completely, but for the time being
    - userland code is more important than not having some dependancy in
    write().

    It's better to stick to cleaner code. Clean code is worth to have.
    As I mentioned in previous, current object based save handler design has
    very
    limited usage and base class part would be better to be removed in the
    future.
    (It's not documented also :)
    And as I previously mentioned - it may be limited or not useful to
    you, but that's just your opinion. Also, it is documented. :)

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Apr 1, 2014 at 12:52 am
    Hi Andrey,
    On Sat, Mar 29, 2014 at 10:47 AM, Andrey Andreev wrote:
    On Sat, Mar 29, 2014 at 3:09 AM, Yasuo Ohgaki wrote:
    Hi Andrey,
    On Sat, Mar 29, 2014 at 8:41 AM, Andrey Andreev wrote:

    You yourself told me that there's no way for the SessionHandler class
    to know if the storage module supports PS_UPDATE_TIMESTAMP_FUNC() or
    not, and that because of that we can't have
    SessionHandler::updateTimestamp().

    It's only applicable for object based user save handler.
    Since user is implemented their own handler, this limitation should not
    matter.
    You're saying this like nobody uses the SessionHandler class. You
    don't know that, it's just an assumption of yours, and based on it
    you're willing to have a limited instead of a complete feature, while
    there are no real technical limitations. This is surprising,
    considering that you cite "lack of API" for other features that are
    IMO not as useful.

    It has very limited usage as I wrote.
    Unless base save handler opens it, it's unusable by its spec. i.e.
    It simply raises errors for not opening base class. Even when base save
    handler open is called, there is no way to handle opened resource in user
    land.

    Due to these 2 limitations, only file based save handler could be extended
    AFAIK.
    In other words, it's useless for other save handlers currently. i.e. 2
    connections
    are required for server based storage. If user must open another
    connection, it's
    better to open by itself.

    When user needs to specialized file save handler, it would better to write
    complete handler. User would not be affected by save handler changes
    with this way, too.

    It's possible to return resources used by C written save handlers, but it's
    not
    good solution. It requires dependency for modules. I don't think such
    dependency
    worth it while user may write complete and more efficient handler in user
    land.

    In addition, there are many resources for a resource. PostgreSQL connection
    can be PDO, pgsql, pq, or even more, for instance.
    Writing data and updating time stamp for GC is distinct feature.

    For file-based sessions - it is, but for i.e. a database, there's not
    much difference. write() does update the timestamp, I see no reason
    why it shouldn't be able to update just the timestamp.

    How to update time stamp is up to save handler and its storage.
    Write and updating time stamp is different operation.
    Memcache does not need it if data is read. RDBMS may have separate
    table to keep track time stamp. For PostgreSQL, updating one field is
    faster than updating whole record.
    I'm not saying it isn't faster to update just one field, I'm saying
    it's still an UPDATE clause in both cases.
    I don't see good reason not to have API for it.
    - Not being able to call parent::updateTimestamp() is a good reason.
    - Having the completely unnecessary SessionUpdateTimestampInterface
    is a good reason.
    - Breaking the design of current userland implementations is a good
    reason.
    - Not being able to have lazy_write (a performance improvement, I
    remind you) by default is a good reason.

    I think current design of object based save handler is better to be
    redesigned.

    Current object based save handler registers "previous" save handler as its
    base.
    However, it's mostly useless without calling parent open() function. i.e.
    Other
    calls simply fails when it does not.

    When open() is called, some resource, e.g. file handle, db connection, etc,
    is
    opened. These resources cannot be accessed from user land, since it's not
    "PHP resource", but raw resource. Thus it's only useful for file based
    storage.
    If user is using their own file based storage for some reason, they are
    better
    to implement their own handler fully.

    I would like to remove and clean up this in future release.
    You don't know how it's used. A user may just prepend the session_id
    before calling parent::, or inject logging into the logic, or whatever
    - people do all kinds of crazy stuff.
    SID extension is not documented.

    User may simply call session_id() before session_start() for this.
    It's simpler and more efficient.

    Besides modular design, if write() and updateTimestamp() are
    merged,
    flag
    parameter
    for write() should be added. It breaks compatibility with current
    save
    handlers. I don't
    want BC that could be avoided also.
    No it shouldn't, the decision whether to write or just update the
    timestamp is based on an internal flag, or on $session_data. No
    additional parameters are required.

    I removed PS(id) dependency from s_read() with new patch as planned.
    Why
    should I introduce new dependency to s_write(), i.e. sub module, that
    breaks
    design? It does not make sense.
    Convenience and consistency in the userland APIs is more important
    than "breaking" internal design. I understand you want to stick to
    best practices, but those practices are not a religion and sometimes
    you need to break them. If you ask me, a lot of things in ext/session
    are broken by design ... creating files from inside read() is one
    example, all methods exposed to userland to be in an interface is
    another. I agree, it's not perfect, but that's just what happens when
    we introduce new features into something that was never designed with
    possible changes in the future in mind. For PHP6, we'll have the
    freedom to redesign the whole thing completely, but for the time being
    - userland code is more important than not having some dependancy in
    write().

    It's better to stick to cleaner code. Clean code is worth to have.
    As I mentioned in previous, current object based save handler design has
    very
    limited usage and base class part would be better to be removed in the
    future.
    (It's not documented also :)
    And as I previously mentioned - it may be limited or not useful to
    you, but that's just your opinion. Also, it is documented. :)

    http://jp1.php.net/manual/en/class.sessionhandler.php

    I overlooked this. If you find doc, it would be great if you could provide
    link.
    There would be a note if base class support is removed.

    Just providing interface is good enough as base class is unusable mostly
    and less efficient. Making base class usable does not seem to worth the
    effort. I think you'll understand what I mean if you try to write it by
    yourself.

    BTW, you're arguing unlocked session is dangerous. To make base class
    usable, user save handler must ignore or disable locking. I think unlocked
    session is useful, but you are considering unlocked session is harmful,
    aren't you?

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Andrey Andreev at Apr 1, 2014 at 9:53 am
    Hi,

    Yasuo, I don't agree with one bit of what you're saying here, so I
    won't even bother replying ... this argument is getting us nowhere.

    I'd like to get this into voting now and I just need to know 1 thing,
    is 'read_only' being renamed to 'read_and_close' or not? If it's being
    renamed anyway, I'd rather not include it in the RFC.

    Cheers,
    Andrey.
  • Andrey Andreev at Apr 3, 2014 at 9:15 am

    On Tue, Apr 1, 2014 at 12:53 PM, Andrey Andreev wrote:
    Hi,

    Yasuo, I don't agree with one bit of what you're saying here, so I
    won't even bother replying ... this argument is getting us nowhere.

    I'd like to get this into voting now and I just need to know 1 thing,
    is 'read_only' being renamed to 'read_and_close' or not? If it's being
    renamed anyway, I'd rather not include it in the RFC.

    Cheers,
    Andrey.
    I guess this commit answers my question:
    https://github.com/yohgaki/php-src/commit/9ba96e6eb21bb52d7bda5279381911e3a75c3497

    I'll remove that part of the RFC and move it to a vote shortly.

    Cheers,
    Andrey.
  • Andrey Andreev at Apr 9, 2014 at 3:40 pm
    Hi all,
    On Thu, Apr 3, 2014 at 12:14 PM, Andrey Andreev wrote:
    I'd like to get this into voting now and I just need to know 1 thing,
    is 'read_only' being renamed to 'read_and_close' or not? If it's being
    renamed anyway, I'd rather not include it in the RFC.

    Cheers,
    Andrey.
    I guess this commit answers my question:
    https://github.com/yohgaki/php-src/commit/9ba96e6eb21bb52d7bda5279381911e3a75c3497

    I'll remove that part of the RFC and move it to a vote shortly.

    Cheers,
    Andrey.
    Sorry about the delay, I was busy during the last few days. I've just
    updated the RFC by removing read_only-related content, because it was
    renamed. While I figure out how to start the vote (first time doing
    it), I'm giving you all a few hours for last-minute comments. :)

    https://wiki.php.net/rfc/session-read_only-lazy_write

    Cheers,
    Andrey.
  • Yasuo Ohgaki at Mar 28, 2014 at 7:35 am
    Hi Julien,
    On Wed, Mar 26, 2014 at 7:29 PM, Julien Pauli wrote:

    In order to avoid further arguments about whether a separate function
    for read-and-close is better or not, I've added an alternative
    proposal - to rename the option to 'read_close' or 'read_and_close'.
    After all, the most important thing is that it's not 'read_only'.

    I agree "read_and_close" is much better discribing what it really does , so
    I prefer it.

    I'm not sure if it's good to have "and" or not, but I'm OK with or without
    "and".

    Should I change it now?
    I mean in my github repo.
    I haven't committed the RFC patch yet.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Julien Pauli at Mar 28, 2014 at 9:15 am

    On Fri, Mar 28, 2014 at 8:34 AM, Yasuo Ohgaki wrote:

    Hi Julien,
    On Wed, Mar 26, 2014 at 7:29 PM, Julien Pauli wrote:

    In order to avoid further arguments about whether a separate function
    for read-and-close is better or not, I've added an alternative
    proposal - to rename the option to 'read_close' or 'read_and_close'.
    After all, the most important thing is that it's not 'read_only'.

    I agree "read_and_close" is much better discribing what it really does ,
    so
    I prefer it.

    I'm not sure if it's good to have "and" or not, but I'm OK with or without
    "and".

    Should I change it now?
    I mean in my github repo.
    I haven't committed the RFC patch yet.
    Yes please.

    Also, for error raising, I saw your github discussion.
    We already raise errors in session functions when the session state is not
    the good one at some point.
    I suggest we do it as well for new session functions that's been introduced
    : session_reset() and session_abort().
    Leave session_write_close() as it is.

    Thx.

    Julien
  • Yasuo Ohgaki at Mar 28, 2014 at 9:19 am
    Hi Julien,
    On Fri, Mar 28, 2014 at 6:15 PM, Julien Pauli wrote:

    Yes please.

    Also, for error raising, I saw your github discussion.
    We already raise errors in session functions when the session state is not
    the good one at some point.
    I suggest we do it as well for new session functions that's been
    introduced : session_reset() and session_abort().
    Leave session_write_close() as it is.
    OK. I'll update my github soon.
    When you think it's ready to merge, please let me know or please merge it.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Julien Pauli at Mar 28, 2014 at 9:27 am

    On Fri, Mar 28, 2014 at 10:19 AM, Yasuo Ohgaki wrote:

    Hi Julien,
    On Fri, Mar 28, 2014 at 6:15 PM, Julien Pauli wrote:

    Yes please.

    Also, for error raising, I saw your github discussion.
    We already raise errors in session functions when the session state is
    not the good one at some point.
    I suggest we do it as well for new session functions that's been
    introduced : session_reset() and session_abort().
    Leave session_write_close() as it is.
    OK. I'll update my github soon.
    When you think it's ready to merge, please let me know or please merge it.
    I'd like we finish the debatte for those two functions (session_abort() and
    session_reset()). Those functions have not been RFC'ed from what I know.
    And Andrey.A seems not to agree very well with them, or at least
    session_reset().

    As soon as debatte is finished, and we found a solution, then we'll merge
    the ideas.

    All other stuff should be done under RFC. Session module is critical, and I
    would not want we merge badness or wrong stuff to 5.6 about it. If done, we
    will revert anyway to have a stable API for the upcoming 5.6
    All other ideas should go to 6.0 branch (to be branched soon).

    Thx.

    Julien
  • Bill Salak at Mar 26, 2014 at 9:50 pm

    In order to avoid further arguments about whether a separate function for read-and-close is better or not,
    I've added an alternative proposal - to rename the option to 'read_close' or 'read_and_close'.
    After all, the most important thing is that it's not 'read_only'.
    Hi Andrey,
    I don't expect to change your mind on the option read_only needing to be changed and frankly I'm not
    really all that concerned about what it's going to be called, but since it's been mentioned several times that
    read only has a commonly understood meaning I supply this for voter consideration:
    ----
    # vim -R ~somefile~
    :help 'readonly'

    # man vim
    -R Read-only mode. The 'readonly' option will be set. You can still edit the buffer, but will be prevented
    from accidently overwriting a file.
    ---
    This is only 1 example that's analogous to opening a session as read only, I'm sure there's many more
    lurking in my subconscious that causes me to think it's an intuitive name for what it does.

    If I didn't know already, when presented with the option for "read_and_close" I would probably have a
    minor wtf moment but it *would* make me read the docs to understand what exactly it means to me if I
    use it, since it's an unconventional option name.
  • Sanford Whiteman at Mar 27, 2014 at 5:34 am

    This is only 1 example that's analogous to opening a session as read
    only
    Is it, though?

    Vim's read-only mode prevents you from silently saving changes to the
    original file, but it does not prevent you from saving changes once
    you confirm them.

    Vim ro is like session_start( read_only = true ) and then something
    like session_write_close( confirm_write = true ) -- an extra arg that
    forces you to confirm the write but allows you to do it if you insist,
    without manually reopening.

    Of course interactive warnings and unattended code can't really be
    compared. But the "continue, if you're sure" experience of Vim ro
    surely isn't the "start over, you screwed up" expected from read_only
    = true.
    If I didn't know already, when presented with the option for
    "read_and_close" I would probably have a minor wtf moment but it
    *would* make me read the docs to understand what exactly it means to
    me if I use it, since it's an unconventional option name.
    I think that you've made a ringing endorsement of an explicit,
    unambiguous name.

    No robust language can seek to be self-explanatory, but it can aim for
    clear, complete documentation. This term has multiple meanings that
    are all "mainstream"; debate about THE meaning is fruitless. IMO.

    -- Sandy

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedMar 24, '14 at 8:48a
activeApr 9, '14 at 3:40p
posts29
users5
websitephp.net

People

Translate

site design / logo © 2022 Grokbase