Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:773
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Tue, 8 Apr 2003 02:27:59 +0300 (EEST)
Sender: jani@cs78146114.pp.htv.fi
Reply-To: Jani Taskinen <sniper@iki.fi>
To: SQUILLACE MASSIMO <msquillace@sogei.it>
cc: internals@lists.php.net
In-Reply-To: <7410A5117CDC9C4CA07FBC870D65A50D1B4BB0@MAILBOX01.domus.ad.sogei.it>
Message-ID: <Pine.LNX.4.44.0304080227230.13719-100000@cs78146114.pp.htv.fi>
References: <7410A5117CDC9C4CA07FBC870D65A50D1B4BB0@MAILBOX01.domus.ad.sogei.it>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Subject: Re: [PHP-DEV] [PATCH] OCI8 module - proposed patches (PART 1 of 2)
From: sniper@iki.fi (Jani Taskinen)


    Nice patches, but could you please provide ones without
    the whitespace changes? Now it's not very clear what you
    have actually changed..
    
    --Jani
    

On Mon, 7 Apr 2003, SQUILLACE MASSIMO wrote:

>Hi, I had to split the original message in two e-mails because it was
>longer than 30000 bytes.
>In this first message you'll find the commentary and the first
>attachment, the second message contains the second attachment.
> 
>Massimo
> 
>------------------------------------------------------------------------
>------------------------------------------------
> 
>Hi all, 
> 
>since posting bug #22674 resulted in no feedback to date and I believe
>OCI8 persistent session management should  quickly be fixed, I studied
>the sources and came up with a solution to that particular bug and more.
> 
>The solutions have been working for a week now on two PHP 4.2.2 and two
>PHP 4.3.1 servers, so this weekend I downloaded the current CVS snapshot
>and applied them there.
> 
>Here follows a description of the problems and the proposed patches:
> 
>1) [Solves bug #22674, part 1] The sequence OCIPLOGON('user1', 'pwd1',
>'db1') + OCILOGON('user2', 'pwd2', 'db1') creates two persistent session
>handles instead of one, because the OCILOGON session wrongly "inherits"
>the persistency attribute of the OCIPLOGON session; BOTH sessions can be
>reused.  I fixed one assignment statement in oci_do_connect.
> 
>2) [Solves bug #22674, part 2] The sequence OCIPLOGON + OCILOGON +
>OCINLOGON (same dbname for all, same userid and password for the latter
>2 calls) still creates two persistent session handles instead of one,
>but the session structure set up by OCILOGON is incorrectly freed when
>the module cleans up the OCINLOGON structures, so that while Oracle
>still sees the surplus session PHP cannot reuse it in subsequent
>scripts. This leads to a session build-up with Oracle eventually giving
>up. The problem lies in the way hashed_details is built: it is the same
>for OCILOGON and OCINLOGON, so when zend_hash_del is called during
>cleanup it deletes BOTH structures. I re-defined the hashed_details for
>OCINLOGON using gettimeofday(), so each OCINLOGON structure is now
>unique and different from any OCILOGON structure.
> 
>These first two patches are in the attached twobugs.txt file; if they
>are accepted bug #22674 may be closed.
> 
>I also attach a fourpatches.txt file, which contains the above solutions
>and two more patches I'd like to submit to your attention, since I
>believe they could help make the module more flexible:
> 
>3) More often than not Oracle installations do NOT provide for a
>user-defined "password verification function", so that userid, password
>and dbname are treated case-insensitively by the RDBMS, while the OCI8
>module generates hashed_details that are case-sensitive. If the
>programmers mix case during development this leads to an excessive
>number of persistent sessions to Oracle. I introduced a php_ini boolean
>variable credentials_toupper which, when true (defaults to false),
>triggers uppercase conversion of userid, password and dbname before
>their use in oci_do_connect.
> 
>4) When the web servers are behind a firewall in a DMZ, the firewall
>truncates sessions when they reach a configurable but unavoidable
>inactivity timeout. This invalidates persistent sessions and in my
>experience (if the firewall doesn't specifically support SQL*NET) can't
>always be circumvented by a decrease in the tcp_keepalive_time; also,
>I'd rather leave such parameters at their default and let the firewall
>do its work which is dictated by security concerns. Idle persistent
>sessions are useless by themselves and should anyway be closed as soon
>as feasible to minimize load on the RDBMS. In past client-server (OS/2)
>applications I came up with a thread-based solution which automatically
>closed the client's Oracle connection after a configurable inactivity
>timeout, but since I don't know if this should be done in the current
>PHP implementation I simply decided to add a timestamp field in the
>oci_server structure and check in the PHP_RINIT_FUNCTION if a php_ini
>configurable connection_timeout (defaults to -1, or no timeout) has
>expired. In the affirmative, the expired server handle and all
>associated session handles are correcly closed. Since this happens at
>request init I am sure the check triggers for all PHP scripts, even
>though they don't access the database, as this helps to keep the number
>of connections down. On the other hand a heavily accessed server will
>never timeout persistent connections. My tests indicate that a 300s
>timeout is high enough to guarantee peak Oracle performance while being
>low enough to never create firewall problems. Especially when coupled
>with the solutions to bug #22674, the overhead induced by the up-front
>check is negligible.
> 
>As stated above, two new php_ini entries are proposed as follows:
> 
>[OCI8]
>;if true, internally convert userid, password and dbname to uppercase
>;oci8.credentials_toupper = false
> 
>;inactivity timeout for persistent connections (seconds). -1 means no
>timeout.
>;oci8.connection_timeout = -1
>
>Hope this helps
> 
>Massimo Squillace
>

-- 
<- For Sale! ->