Bug 15947 - close inherited filedescriptors
Summary: close inherited filedescriptors
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
: 10777 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-05-15 15:29 UTC by Colin Walters
Modified: 2008-05-30 17:51 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
don't inherit fds (722 bytes, patch)
2008-05-15 15:29 UTC, Colin Walters
Details | Splinter Review
don't inherit fds (removed if check) (703 bytes, patch)
2008-05-15 15:37 UTC, Markus Rechberger
Details | Splinter Review
using posix sysconf(_SC_OPEN_MAX) instead getdtablesize (709 bytes, patch)
2008-05-21 09:09 UTC, Markus Rechberger
Details | Splinter Review

Description Colin Walters 2008-05-15 15:29:06 UTC
From: Markus Rechberger <mrechberger@empiatech.com>

Hi,

when the dbus library spawns out dbus-launch it
inherits all the filedescriptors from the parent.
This causes problems in several ways for example when
the inherited file is a devicenode, it won't be closed
until dbus gets shut down.
As for multimedia devices when mplayer starts up and
opens for example the video4linux node dbus will
inherit that filedescriptor and the device will never
be closed. For devices which support multiple
devicemodes this lock the device in the certain mode.

Attached a patch which fixes that problem.
Comment 1 Colin Walters 2008-05-15 15:29:54 UTC
Created attachment 16558 [details] [review]
don't inherit fds
Comment 2 Markus Rechberger 2008-05-15 15:37:12 UTC
Created attachment 16559 [details] [review]
don't inherit fds (removed if check)
Comment 3 Colin Walters 2008-05-15 15:44:27 UTC
I was about to follow up on that =) 

Looks like a reasonable patch to me.
Comment 4 Havoc Pennington 2008-05-15 15:56:45 UTC
Comment on attachment 16559 [details] [review]
don't inherit fds (removed if check)

This should look more like in glib/gspawn.c ; should use fdwalk() if available, and probably sysconf() or getrlimit() would be more modern than getdtablesize().

Can't just cut-and-paste glib code though, due to license. Maybe the author of that code would be willing to relicense, though it is not complex to reimplement.
Comment 5 Markus Rechberger 2008-05-21 09:09:57 UTC
Created attachment 16665 [details] [review]
using posix sysconf(_SC_OPEN_MAX) instead getdtablesize
Comment 6 Colin Walters 2008-05-23 16:33:26 UTC
I had a quick look at how Python's subprocess module does it; it basically uses sysconf to get the max fds, and then calls a new function "os.closerange", which  is just a basic loop doing close() over the fd range.

fdwalk() seems like a Solaris-specific function?  I would say let a patch for that be written/tested by someone from the OpenSolaris community if they want it.

Stylistically this patch doesn't match the rest of the code - need a space between name and function call and the for loop.

After that's fixed r=me.


Comment 7 Colin Walters 2008-05-28 13:31:50 UTC
commit e2bc7232069b14b7299cb8b2eab436f60a232007
Author: Colin Walters <walters@verbum.org>
Date:   Wed May 28 16:01:22 2008 -0400

    Bug 15947: Close file descriptors before execing helper (Markus Rechberger)
    
    	* dbus/dbus-sysdeps-unix.c (_dbus_get_autolaunch_address):
    	Close file descriptors before exec.

Comment 8 Colin Walters 2008-05-30 17:51:51 UTC
*** Bug 10777 has been marked as a duplicate of this bug. ***


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.