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.
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);
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<[email protected]>
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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
