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

[5.3] Fix empty session #15998

Merged
merged 1 commit into from
Oct 24, 2016
Merged

[5.3] Fix empty session #15998

merged 1 commit into from
Oct 24, 2016

Conversation

lcharette
Copy link
Contributor

Fix session being empty when the file is being read and/or written by multiple instance of apache simultaneously. This append to me when assets (javascript / css) are being loaded dynamically using php. Multiples assets in the same request cause the sessions to be corrupted.

Fix session being empty when the file is being read and/or written by multiple instance of apache simultaneously. This append to me when assets (javascript / css) are being loaded dynamically using php. Multiples assets in the same request cause the sessions to be corrupted.
@taylorotwell
Copy link
Member

@GrahamCampbell @themsaid do you know of any problems with this?

@maki10
Copy link
Contributor

maki10 commented Oct 19, 2016

Hey @taylorotwell I have the same issue when using multiple iframe elements. If a session is not in Redis, a session will be random expired.

I can reproduce this issue with my Admin panel that is built for Laravel on a fresh install of Laravel.

@GrahamCampbell GrahamCampbell changed the title Fix empty session [5.3] Fix empty session Oct 19, 2016
@GrahamCampbell
Copy link
Member

do you know of any problems with this?

Looks ok. I guess it doesn't matter that much, since any real usage will be using apc or redis, or the like.

@lcharette
Copy link
Contributor Author

From all the debug and tests I did, the error I encounter was really when the same session file was read/write more than once in a very very short time period (simultaneously). It would randomly return noting. Probably something related to file_get_contents trying to read the file while it's being written.

As seen here, adding the lock option to true force php to get a read lock on the file before reading it's content, assuring the file will indeed be read.

@taylorotwell
Copy link
Member

So adding this change fixed your original problem in your tests?

@lcharette
Copy link
Contributor Author

Yes. Like I said, this force php to get a read lock on the file before reading it's content, assuring the file will indeed be read correctly. So the file read won't return en empty string as the session data, forcing a new session to be created by our system.

@taylorotwell taylorotwell merged commit e9ed079 into laravel:5.3 Oct 24, 2016
@frohlfing
Copy link

it seems to fix my issue #15551, too!

@lcharette
Copy link
Contributor Author

What you describe in #15551 is similar to the problem I had, so good chances this indeed fixed your error too ;)

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

Successfully merging this pull request may close these issues.

5 participants