From dev-return-22943-apmail-apr-dev-archive=apr.apache.org@apr.apache.org Tue Feb 16 17:21:00 2010 Return-Path: Delivered-To: apmail-apr-dev-archive@www.apache.org Received: (qmail 41758 invoked from network); 16 Feb 2010 17:21:00 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 16 Feb 2010 17:21:00 -0000 Received: (qmail 4114 invoked by uid 500); 16 Feb 2010 17:21:00 -0000 Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 4022 invoked by uid 500); 16 Feb 2010 17:20:59 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Id: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 4014 invoked by uid 99); 16 Feb 2010 17:20:59 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Feb 2010 17:20:59 +0000 X-ASF-Spam-Status: No, hits=0.2 required=10.0 tests=RCVD_IN_DNSWL_LOW,SPF_NEUTRAL X-Spam-Check-By: apache.org Received-SPF: neutral (athena.apache.org: local policy) Received: from [213.191.128.81] (HELO mxout2.iskon.hr) (213.191.128.81) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Feb 2010 17:20:50 +0000 Received: from mxscanout.iskon.hr (localhost [127.0.0.1]) by localhost (Postfix) with SMTP id 5F38B27972E for ; Tue, 16 Feb 2010 18:20:28 +0100 (CET) Received: from mx.iskon.hr (unknown [213.191.142.123]) by mxscanout.iskon.hr (Postfix) with SMTP id 477ED2C833D for ; Tue, 16 Feb 2010 18:20:27 +0100 (CET) Received: (qmail 17218 invoked from network); 16 Feb 2010 18:20:27 +0100 X-Remote-IP: 89.164.32.240 Received: from 32-240.dsl.iskon.hr (HELO fc12x32m0.jboss.hr) (89.164.32.240) by mx.iskon.hr with SMTP; 16 Feb 2010 18:20:27 +0100 Message-ID: <4B7AD3D8.5020609@apache.org> Date: Tue, 16 Feb 2010 18:20:24 +0100 From: Mladen Turk User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.7) Gecko/20100120 Fedora/3.0.1-1.fc12 Lightning/1.0b1 Thunderbird/3.0.1 MIME-Version: 1.0 To: dev@apr.apache.org Subject: Re: svn commit: r910597 - /apr/apr/trunk/shmem/unix/shm.c References: <20100216170710.760BD23888CE@eris.apache.org> In-Reply-To: <20100216170710.760BD23888CE@eris.apache.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: IskonProtect X-PerlMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' BODY_SIZE_1000_LESS 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, BODY_SIZE_700_799 0, TO_NO_NAME 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __HAS_MSGID 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MOZILLA_MSGID 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __URI_NS , __USER_AGENT 0' On 02/16/2010 06:07 PM, jfclere@apache.org wrote: > Log: > Make sure we don't leak file descriptors. > > if (new_m->shmkey == (key_t)-1) { > + apr_file_close(file); > return errno; > } File will be closed when the pool gets destroyed. Closing here has little advantages cause the pool memory will rise over time so you cannot be sure the provided pool won't get dirty if open fails. It would be better to create a sub pool for shared memory which will ensure that both parent pool doesn't leak memory and that file is always closed. Repeating apr_file_close across all possible return failures just make no sense when we have pools. Regards -- ^TM