From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18319 invoked by alias); 21 Mar 2007 22:31:48 -0000 Received: (qmail 18304 invoked by uid 22791); 21 Mar 2007 22:31:44 -0000 X-Spam-Check-By: sourceware.org Received: from smtp1.dnsmadeeasy.com (HELO smtp1.dnsmadeeasy.com) (205.234.170.134) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 21 Mar 2007 22:31:33 +0000 Received: from smtp1.dnsmadeeasy.com (localhost [127.0.0.1]) by smtp1.dnsmadeeasy.com (Postfix) with ESMTP id 75D9B29A7E9; Wed, 21 Mar 2007 22:31:31 +0000 (GMT) X-Authenticated-Name: js.dnsmadeeasy X-Transit-System: In case of SPAM please contact abuse@dnsmadeeasy.com Received: from avtrex.com (unknown [67.116.42.147]) by smtp1.dnsmadeeasy.com (Postfix) with ESMTP; Wed, 21 Mar 2007 22:31:31 +0000 (GMT) Received: from [192.168.7.109] ([192.168.7.109]) by avtrex.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 21 Mar 2007 15:31:30 -0700 Message-ID: <4601B242.5010207@avtrex.com> Date: Wed, 21 Mar 2007 22:31:00 -0000 From: David Daney User-Agent: Thunderbird 1.5.0.10 (X11/20070302) MIME-Version: 1.0 To: Andrew Haley Cc: java-patches@gcc.gnu.org, tromey@redhat.com Subject: Re: [Patch] PR 31228, Fix close-on-exec race. References: <45FE35C1.9060805@avtrex.com> <46005416.1070700@avtrex.com> <460058DA.7090300@avtrex.com> <46016C9D.4000909@avtrex.com> <17921.28992.610579.267692@zebedee.pink> In-Reply-To: <17921.28992.610579.267692@zebedee.pink> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact java-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: java-patches-owner@gcc.gnu.org X-SW-Source: 2007-q1/txt/msg00704.txt.bz2 Andrew Haley wrote: > David Daney writes: > > Tom Tromey wrote: > > >>>>>> "David" == David Daney writes: > > >>>>>> > > > > > > David> Make the accept non-blocking, and do a select/poll on the > > > David> ServerSocket. That way the accept would never block and you could use > > > David> a mutex. > > > > > > Yeah. I hadn't thought of the accept case, thanks. > > > > > > David> This makes me think that ld.so is probably broken also. When we do > > > David> dlopen ld.so opens the library file and does several mmaps before > > > David> closing the file descriptor. These could leak out as well. > > > > > > Not to mention other places that libc may create fds. > > > > > > On some platforms fdwalk(3) can be used to make this close loop more > > > efficient. There's also closefrom(3). Linux doesn't seem to have > > > either of these, but Solaris does. > > > > > > Tom > > > > > How about this version? > > This is impressively heroic, but I really think you should measure the > performance improvement before committing this. :-) > > Andrew. > Hard data takes the fun out of things :-( Here is the test program: I open 50 files and then Runtime.exec("/bin/true") 10,000 times. ------------------------------------------------------------------------ import java.io.*; import java.util.*; public class ExecTest { public static void main(String args[]) { try { ArrayList l = new ArrayList(); for (int i = 0; i < 50; i++) { FileOutputStream fos = new FileOutputStream("junk" + i); l.add(fos); } Runtime r = Runtime.getRuntime(); long startTime = System.nanoTime(); for (int i = 0; i < 10000; i++) { Process p = r.exec("/bin/true"); if(false) { InputStream is = p.getInputStream(); byte buffer[] = new byte[1024]; for (;;) { int nr = is.read(buffer); if (-1 == nr) break; System.out.write(buffer, 0, nr); } } p.waitFor(); } long endTime = System.nanoTime(); System.out.println("Elapsed: " + (endTime - startTime)); } catch (Exception ex) { ex.printStackTrace(); } } } ------------------------------------------------------ I did three test runs of each of the following: 1) ulimit -n 1024; A corrected version of my last patch that gets a directory of /proc/self/fd and closes all of the descriptors found. 19158418000 19570065000 19396004000 2) ulimit -n 1024; close *all* descriptors up to the ulimit -n value. 16650209000 17505032000 16257549000 3) ulimit -n 10240; same code as case 2. 28571639000 28423899000 27640995000 4) ulimit -n 100; same code as case 2. 14759709000 16140129000 15765422000 What does all this mean? Well on my x86_64-pc-linux-gnu notebook, it takes about 1 second to close 10,000,000 times an invalid file descriptor. The savings of only closing open files would appear to make sense only if the default ulimit -n value is raised to a value greater than about 3000. The only reason to increase ulimit -n, from its default value of 1024, is if you expect to have a lot of open files. In this case This is all moot as the time to close the open files would be much greater than the overhead of closing invalid descriptors. So if we want to fix the bug, I would recommend my original patch. David Daney