Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:71772
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.52 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <18069656.FoN8fZuO6C@rofl>
References: <CAGa2bXZEXx7H+atr3LK8OLLQGSd9a-6+Gey+RhPwP0d7EvTBmw@mail.gmail.com>
 <52E9A631.5050808@sugarcrm.com> <CAGa2bXYn8i9R2+FGMUeyrr00MjRMug5QhheLa=gLKSOc+0j-CA@mail.gmail.com>
 <18069656.FoN8fZuO6C@rofl>
Date: Thu, 30 Jan 2014 14:34:35 +0900
Message-ID: <CAGa2bXYERArF7-BAwyoBevRGcoqN+GS1tF0NHCq6rNigAvN_+A@mail.gmail.com>
To: Patrick Schaaf <php@bof.de>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11336d900f6f3c04f1296cee
Subject: Re: [PHP-DEV] Re: [VOTE] Introduce session.lock, session.lazy_write
 and session.lazy_destory
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a11336d900f6f3c04f1296cee
Content-Type: text/plain; charset=UTF-8

Hi Patrick,

On Thu, Jan 30, 2014 at 1:41 PM, Patrick Schaaf <php@bof.de> 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
yohgaki@ohgaki.net

--001a11336d900f6f3c04f1296cee--