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

[HttpFoundation][Session] Fix memcache session handler #20375

Merged
merged 1 commit into from Nov 4, 2016

Conversation

klandaika
Copy link
Contributor

@klandaika klandaika commented Nov 1, 2016

Q A
Branch? 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, 2.8, 3.0, 3.1, master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Commit 0216e05 removed the opening of connection to memcached server on call to open(), because it's assumed that connection is already opened. However, close() still closes the connection. As a result no more read/write calls can be made if session got closed, as the connection does not get reestablished.

Basically MemcacheSessionHandler should follow same logic as MemcachedSessionHandler, which is exactly what this MR acomplishes.

@linaori
Copy link
Contributor

linaori commented Nov 2, 2016

You should pick 2.7 as base branch for bug fixes, as the others are not supported any more. It could then possibly be merged upwards to 2.8, 3.1 and dev-master/3.2-dev

@klandaika klandaika changed the base branch from 2.1 to 2.7 November 2, 2016 12:48
@klandaika
Copy link
Contributor Author

Changed the base to 2.7 as requested.

@fabpot
Copy link
Member

fabpot commented Nov 2, 2016

sounds reasonnable

@fabpot
Copy link
Member

fabpot commented Nov 4, 2016

Thank you @klandaika.

@fabpot fabpot merged commit 0423d89 into symfony:2.7 Nov 4, 2016
fabpot added a commit that referenced this pull request Nov 4, 2016
…andaika)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpFoundation][Session] Fix memcache session handler

| Q             | A
| ------------- | ---
| Branch?       | 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, 2.8, 3.0, 3.1, master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Commit 0216e05 removed the opening of connection to memcached server on call to `open()`, because it's assumed that connection is already opened. However, `close()` still closes the connection. As a result no more read/write calls can be made if session got closed, as the connection does not get reestablished.

Basically MemcacheSessionHandler should follow same logic as Memcache**d**SessionHandler, which is exactly what this MR acomplishes.

Commits
-------

0423d89 [HttpFoundation][Session] memcached connection should not be closed
@klandaika
Copy link
Contributor Author

Do I have to do anything to get this merged up to master? or do you handle that internally?

@fabpot
Copy link
Member

fabpot commented Nov 4, 2016

Nothing to do on your side, we merge old branches into newer ones on a regular basis. Thanks.

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

Successfully merging this pull request may close these issues.

None yet

4 participants