Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:10585
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: internals@lists.php.net
Date: Fri, 18 Jun 2004 14:13:05 +0200
User-Agent: KMail/1.6.2
Cc: Joe Orton <jorton@redhat.com>
References: <20040618103032.GA25820@redhat.com>
In-Reply-To: <20040618103032.GA25820@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed;
  protocol="application/pgp-signature";
  micalg=pgp-sha1;
  boundary="Boundary-03=_Zxt0A90YCqMWOmV";
  charset="utf-8"
Content-Transfer-Encoding: 7bit
Message-ID: <200406181413.14024.magnus@php.net>
Subject: Re: [PHP-DEV] [PATCH] apache2handler memory leak fix
From: magnus@php.net (Magnus =?utf-8?q?M=C3=A4=C3=A4tt=C3=A4?=)

--Boundary-03=_Zxt0A90YCqMWOmV
Content-Type: multipart/mixed;
  boundary="Boundary-01=_Rxt0AMoYBcJFQGW"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

--Boundary-01=_Rxt0AMoYBcJFQGW
Content-Type: text/plain;
  charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Hi!

On Friday 18 June 2004 12.30, Joe Orton wrote:
> It's simpler to just use the ap_r* interfaces in the the handler SAPI
> for 2.0, this improves network usage by allowing httpd to buffer as
> necessary, and fixes a bug where ub_write is unnecessarily pmemdup'ing
> the string (it could have used a transient bucket to avoid that; the
> apache2filter got this right), and stops re-using an output brigade
> which is now or at least will soon be a 2.0 API violation.
>
> (this restores constant memory use for an arbitrary length response)
>

I've attached a forward port of this patch for HEAD.
This also fixes bug #28113.

Hopefully I didn't screw anything up.

/Magnus


=2D-=20
In California they don't throw their garbage away -- they make it into
television shows.
		-- Woody Allen, "Annie Hall"

--Boundary-01=_Rxt0AMoYBcJFQGW
Content-Type: text/plain;
  charset="utf-8";
  name="apache2handler.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="apache2handler.diff"

Index: sapi/apache2handler/sapi_apache2.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /repository/php-src/sapi/apache2handler/sapi_apache2.c,v
retrieving revision 1.39
diff -u -r1.39 sapi_apache2.c
=2D-- sapi/apache2handler/sapi_apache2.c	18 Jun 2004 00:36:58 -0000	1.39
+++ sapi/apache2handler/sapi_apache2.c	18 Jun 2004 12:07:47 -0000
@@ -48,7 +48,7 @@
 #include "http_log.h"
 #include "http_main.h"
 #include "util_script.h"
=2D#include "http_core.h"                        =20
+#include "http_core.h"
 #include "ap_mpm.h"
=20
 #include "php_apache.h"
@@ -58,7 +58,7 @@
  * file does not use the system call shutdown, it is safe to #undef it.K
  */
 #undef shutdown
=2D=20
+
 #define PHP_MAGIC_TYPE "application/x-httpd-php"
 #define PHP_SOURCE_MAGIC_TYPE "application/x-httpd-php-source"
 #define PHP_SCRIPT "php5-script"
@@ -69,31 +69,16 @@
 static int
 php_apache_sapi_ub_write(const char *str, uint str_length TSRMLS_DC)
 {
=2D	apr_bucket *bucket;
=2D	apr_bucket_brigade *brigade;
 	request_rec *r;
 	php_struct *ctx;
=2D	char *copy_str;
=2D
=2D	if (str_length =3D=3D 0) {
=2D		return 0;
=2D	}
=20
 	ctx =3D SG(server_context);
 	r =3D ctx->r;
=2D	brigade =3D ctx->brigade;
=2D=09
=2D	copy_str =3D apr_pmemdup(r->pool, str, str_length);
=2D	bucket =3D apr_bucket_pool_create(copy_str, str_length, r->pool, r->con=
nection->bucket_alloc);
=2D						=20
=2D	APR_BRIGADE_INSERT_TAIL(brigade, bucket);
=20
=2D	if (ap_pass_brigade(r->output_filters, brigade) !=3D APR_SUCCESS || r->=
connection->aborted) {
+	if (ap_rwrite(str, str_length, r) < 0) {
 		php_handle_aborted_connection();
 	}
=2D	/* Ensure this brigade is empty for the next usage. */
=2D	apr_brigade_cleanup(brigade);
=2D=09
+
 	return str_length; /* we always consume all the data passed to us. */
 }
=20
@@ -241,8 +226,6 @@
 php_apache_sapi_flush(void *server_context)
 {
 	php_struct *ctx;
=2D	apr_bucket_brigade *brigade;
=2D	apr_bucket *bucket;
 	request_rec *r;
 	TSRMLS_FETCH();
=20
@@ -255,20 +238,15 @@
 	}
=20
 	r =3D ctx->r;
=2D	brigade =3D ctx->brigade;
=20
 	sapi_send_headers(TSRMLS_C);
=20
 	r->status =3D SG(sapi_headers).http_response_code;
 	SG(headers_sent) =3D 1;
=20
=2D	/* Send a flush bucket down the filter chain. */
=2D	bucket =3D apr_bucket_flush_create(r->connection->bucket_alloc);
=2D	APR_BRIGADE_INSERT_TAIL(brigade, bucket);
=2D	if (ap_pass_brigade(r->output_filters, brigade) !=3D APR_SUCCESS || r->=
connection->aborted) {
+	if (ap_rflush(r) < 0 || r->connection->aborted) {
 		php_handle_aborted_connection();
 	}
=2D	apr_brigade_cleanup(brigade);
 }
=20
 static void php_apache_sapi_log_message(char *msg)
@@ -277,7 +255,7 @@
 	TSRMLS_FETCH();
=20
 	ctx =3D SG(server_context);
=2D  =20
+
 	/* We use APLOG_STARTUP because it keeps us from printing the
 	 * data and time information at the beginning of the error log
 	 * line.  Not sure if this is correct, but it mirrors what happens
@@ -504,8 +482,8 @@
 	}
=20
 	/* Setup the CGI variables if this is the main request */
=2D	if (r->main =3D=3D NULL ||=20
=2D		/* .. or if the sub-request envinronment differs from the main-request=
=2E */=20
+	if (r->main =3D=3D NULL ||
+		/* .. or if the sub-request envinronment differs from the main-request. =
*/
 		r->subprocess_env !=3D r->main->subprocess_env
 	) {
 		/* setup standard CGI variables */

--Boundary-01=_Rxt0AMoYBcJFQGW--

--Boundary-03=_Zxt0A90YCqMWOmV
Content-Type: application/pgp-signature
Content-Description: signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.9.8 (GNU/Linux)
Comment: 

iD8DBQBA0txZ8JJ96JyIrNYRAt+ZAKCKO6srzjCo45ZCaOzoglj9LCkRhwCdHJdZ
jjfSWIQq0vfFNqpowRrya0U=
=KK1x
-----END PGP SIGNATURE-----

--Boundary-03=_Zxt0A90YCqMWOmV--