Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:44503
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain prohost.org from 74.125.92.26 cause and error)
Cc: Pierre Joye <pierre.php@gmail.com>,
 Lukas Kahwe Smith <mls@pooteeweet.org>,
 PHP internals <internals@lists.php.net>
Message-ID: <901C4626-24FD-46A6-A116-7B9502FCD67D@prohost.org>
To: Rasmus Lerdorf <rasmus@lerdorf.com>
In-Reply-To: <4A454583.9000801@lerdorf.com>
Content-Type: multipart/mixed; boundary=Apple-Mail-68--682284075
Mime-Version: 1.0 (Apple Message framework v935.3)
Date: Fri, 26 Jun 2009 19:52:51 -0400
References: <9A46F4B8-E64A-4C3C-B2A5-FC354A3EB71D@pooteeweet.org>	 <0C2F23C2-D188-4938-B44C-4ED166B934CE@macvicar.net>	 <fe05d1540906261126k2d6f12fenf5b00b44044614d4@mail.gmail.com>	 <EAD7D00F-5B9F-4D43-9614-589F5BE039D8@pooteeweet.org>	 <4A451D6A.8090102@lerdorf.com> <fe05d1540906261218j7aec2454v80c9637e4a98e6a@mail.gmail.com> <4A454583.9000801@lerdorf.com>
Subject: Re: [PHP-DEV] post 5.3.0 development
From: ilia@prohost.org (Ilia Alshanetsky)

--Apple-Mail-68--682284075
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: 7bit

Here is the least intrusive fix I can come up for this bug. When doing  
curl_close() the dtor will force flush of data on a file stream  
synching the data to disk. As far as I can tell (using Rasmus'  
example) this appears to adequately fix the problem and I see no  
immediate side-effects.


--Apple-Mail-68--682284075
Content-Disposition: attachment;
	filename=curl.txt
Content-Type: text/plain;
	x-unix-mode=0644;
	name="curl.txt"
Content-Transfer-Encoding: 7bit

Index: ext/curl/interface.c
===================================================================
RCS file: /repository/php-src/ext/curl/interface.c,v
retrieving revision 1.62.2.14.2.57
diff -u -p -a -d -r1.62.2.14.2.57 interface.c
--- ext/curl/interface.c	15 Jun 2009 12:38:11 -0000	1.62.2.14.2.57
+++ ext/curl/interface.c	26 Jun 2009 23:50:00 -0000
@@ -2093,6 +2093,11 @@ static void _php_curl_close_ex(php_curl 
 		efree(ch->header.str);
 	}
 
+	/* flush the file handle, so any remaining data is synched to disk */
+	if (ch->handlers->write->method = PHP_CURL_FILE && ch->handlers->write->fp) {
+		fflush(ch->handlers->write->fp);
+	}
+
 	efree(ch->handlers->write);
 	efree(ch->handlers->write_header);
 	efree(ch->handlers->read);

--Apple-Mail-68--682284075
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: 7bit



Ilia Alshanetsky


On 26-Jun-09, at 6:02 PM, Rasmus Lerdorf wrote:

> Pierre Joye wrote:
>> On Fri, Jun 26, 2009 at 9:11 PM, Rasmus Lerdorf<rasmus@lerdorf.com>  
>> wrote:
>>> Lukas Kahwe Smith wrote:
>>>> Exactly.
>>>> I will do my best to track things that need to be merged. Best is  
>>>> to
>>>> note if something needs to be merged.
>>>>
>>>> But if you all feel it's such a huge burden then you can of course
>>>> insist on putting the burden on the RMs. The fact of the matter  
>>>> is that
>>>> our current infrastructure is not fit for providing both sides  
>>>> with an
>>>> efficient solution.
>>> Keep an eye on http://bugs.php.net/48518
>>>
>>> I don't think we can let 5.3 out the door with this regression.
>>
>> btw, 5.2.10 is out too with this regression. If I'm not mistaken.
>
> Ok, I am running out of time for this one today.
>
> As Tony mentioned in his original bug report for 48518:
>
>  Simple patch fixes this problem, but there is another one to  
> consider:
>  should the refcount be decreased when closing the cURL handle?
>
> That would be a yes.  I don't see an easy way to relate an open
> File-Handle resource back to the curl handle though short of walking  
> all
> open file handles and checking them against the fps in the curl  
> struct,
> or creating a separate llist for these resources tied to the curl
> handle.  If someone has an idea for a clean fix here, please speak up.
>
> Second, because there was no addref on the fp before, a simple
>
>  curl_exec($ch);
>  fclose($fp);
>
> would force a flush and the data would be available in the file.  Even
> if we decrement the refcount on the file handle resource in the curl
> handle destructor, that still doesn't bring us back to that behaviour
> because the fclose there doesn't actually close the file yet, as it
> shouldn't.  So here, a secondary fix, and a side-effect of this issue,
> is that we should probably force a flush on the fclose for any  
> flushable
> streams we try to close when there are outstanding references holding
> them open.
>
> I think doing both of these should take care of this one.
>
> And 10 years ago I'd be hacking on this for the next 3 hours, but  
> right
> now I am spending the next 3 hours picking up Carl from Lego Camp and
> taking him to his piano lesson...
>
> -Rasmus


--Apple-Mail-68--682284075--