Hi Patrick,
On Thu, Jan 30, 2014 at 1:41 PM, Patrick Schaaf <[email protected]> wrote:
> First of all, the github "compare" link you give in the RFC shows a vast
> amount of unrelated changed files. Playing around a bit I find that
> comparing
> not to php:master but to yohgaki:PHP-5.6 gives a better overview:
>
>
> https://github.com/yohgaki/php-src/compare/PHP-5.6...PHP-5.6-rfc-session-lock
>
> Regarding minimize_lock:
>
> Generally a really dangerous feature. minimize_lock sounds so friendly,
> does
> not imply danger really.
>
> Alternative naming proposal: unlocked_thus_unsafe
>
It sounds good idea. How about shorter name?
unsafe_lock
> To solve all three problems, I'd suggest
>
> - unconditional taking of the flock(LOCK_EX) in ps_files_open(), while
> keeping
> the LOCK_UN in PS_READ_FUNC/PS_WRITE_FUNC (if minimize_lock=true).
>
This would be safer and good for reference implementation.
> - also when minimize_lock=true, make an fstat() call in PS_WRITE_FUNC
> before
> the need-to-ftruncate check, ignoring data->st_size from the read.
>
I'll fix this later in my branch.
> Reading that code, also in the base PHP repository, I think I spotted an
> unrelated bug: In ps_files_open() in the not-WIN32-open_basedir
> fstat/S_ISLNK
> _error_ cases: data->fd is closed, but data->fd is then NOT set to -1.
>
This should be set to -1.
I've committed the fix.
Thank you.
Regards.
--
Yasuo Ohgaki
[email protected]