Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:21115
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Thu, 8 Dec 2005 23:40:13 +0200 (EET)
Reply-To: Jani Taskinen <sniper@iki.fi>
To: David Oren <david_oren@excite.com>
cc: internals@lists.php.net
In-Reply-To: <20051208081523.8777F2999F@xprdmailfe21.nwk.excite.com>
Message-ID: <Pine.LNX.4.61.0512082338480.20989@arfg.argcubovn.sv>
References: <20051208081523.8777F2999F@xprdmailfe21.nwk.excite.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Subject: Re: [PHP-DEV] Bugs in zend.c and zend_compile.c in ZTS mode
From: sniper@iki.fi (Jani Taskinen)


    We always prefer patches over long stories. :)
    And preferrably first to somehow reproduce the problem easily..

    --Jani

On Thu, 8 Dec 2005, David Oren wrote:
>
> Hello all,
>
> I have recently switched to PHP 5.1, and have been encountering crashes
> during shutdown. I have tried delving into the PHP code to locate the
> source of the problems, and have discovered two issues, both of which only
> happen when compiled with ZTS enabled.
>
> The first is very probably a bug.
>
> In zend_compile.c, in the zend_initialize_class_data function, inside the
> #ifdef ZTS block, the following line can be found:
>
>    ce->static_members = (HashTable*)n;
>
> This casts an int variable, which denotes an index of the static_members
> array, to a pointer. I assume what was meant is:
>
>    ce->static_members = (HashTable*) (CG(static_members)[n]);
>
> In addition, support for runtime declaration (handled by the if block) is
> broken, as it will not work when CG(static_members) is NULL and we're
> trying to add the first member, so the if block should be followed by an
> else block similar to this one.
>
>    else if (!CG(static_members)) {
>        // n is probably 0 at this point
>        CG(last_static_member) = n+1;
>        CG(static_members) = (HashTable**)calloc(n+1, sizeof(HashTable*));
>    }
>
> If needed, I can provide a patch for this issue. The second issue _may_ be
> a bug, but I'm not familiar enough with the Zend Engine internals to be
> certain.
>
> In zend.c, the zend_post_startup function unlinks the global (r/o) copies
> of several tables, and creates fresh r/w copies for the startup threads.
>
> From looking at the code, it would seem that the unlinking of the
> persistent_list is NOT done cleanly: The persistent list of the running
> thread is destroyed and reinitialized (by executor_globals_ctor), and this
> is what the global_persistent_list variable points to. In other words, the
> global persistent list is shared with the startup thread.
>
> Again, in this case it _feels_ wrong, but this may also be what was intended.
>
> I hope I've managed to make sense, and I'd appreciate any feedback on this.
>
> Thanks,
>
> David Oren
>