Unless we have reports that this happens IRL, I think we should fix as little as possible. The linked report is only about non-atomic writes. I'd wait before doing more on this topic.

Unless we have reports that this happens IRL,

I felt like the linked comment reported by@mpdude were a real case.

I felt like the linked comment reported by@mpdude were a real case.

It is, and the message there is about "Notice: unserialize()". That's unrelated to theis_file() check you're mentioning for now.

Overhere, I also mentioned that theunlink() call may be a source of errors as well, since the file might have been deleted by another process in the meantime.

OK, thanks for the link. So you have processes that concurrently destroy the session?
That cannot work of course, whatever we do here, there will always be an issue with concurrent requests that read and destroy at the same time.
You should not destroy concurrently to me.

Now updated to guard against concurrentunlink().

Regarding@jderusse's comment above, theis_file($filePath) ? file_get_contents($filePath) concurrency thing could also be addressed with something like

set_error_handler(staticfunction () {});try {$this->data = [];$this->data =unserialize(file_get_contents($filePath));        }finally {restore_error_handler();        }

But regardless of that, 👍 and thanks for the fix!

the is_file($filePath) ? file_get_contents($filePath) concurrency thing

code updated

@nicolas-grekas i just merged your PR into my largest test-set, and it's still passing! Thank you!