landscape-client should close stray inherited file descriptors on startup

Bug #352458 reported by Christopher Armstrong on 2009-03-31
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Landscape Client
Critical
Christopher Armstrong
Landscape Server
Critical
Christopher Armstrong
landscape-client (Ubuntu)
Undecided
Unassigned

Bug Description

During normal startup it's not an issue, but it can become one when landscape-client upgrades itself.

<cjwatson> niemeyer: if a process inherits the fd that's for writing to debconf and doesn't close it on startup, then debconf will hang when it's trying to shut down
<cjwatson> niemeyer: so this isn't for future package installations using smart, it's something that landscape-client needs to do on startup if it's called from a postinst

I have determined that this debconf file descriptor *is* opened in a way that will be inherited by subprocesses, so we really should clean it up. (exec 3>&1)

Changed in landscape-client:
assignee: nobody → radix
milestone: none → 1.0.29
Changed in landscape:
milestone: none → mountainview-pre-8
Changed in landscape:
milestone: mountainview-pre-8 → mountainview
tags: added: review
Thomas Herve (therve) wrote :

[1]
+ min(4096, resource.getrlimit(resource.RLIMIT_NOFILE)[1])):

Can you create an intermediate variable for the getrlimit value?

[2]
+from landscape.tests.helpers import LandscapeTest
+
+class CleanFDsTests(LandscapeTest):

Missing a blank line.

[3]
+ clean_fds()
+
+
+ def test_clean_fds_sanity(self):

Extra blank line :).

+1!

Christopher Armstrong (radix) wrote :

therve:
[1-3] Done!

Changed in landscape-client:
importance: Undecided → Critical
Changed in landscape:
importance: Undecided → Critical
Jamu Kakar (jkakar) wrote :

[1]

The tests pass but I'm getting a weird error:

-------------------------------------------------------------------------------
Ran 1482 tests in 76.271s

PASSED (successes=1482)
Error in sys.exitfunc:

It seems to happen in trunk, too, so I guess it wasn't introduced
here.

+1!

tags: removed: review
Christopher Armstrong (radix) wrote :

Merged to trunk, thanks

Changed in landscape-client:
status: New → Fix Committed
Changed in landscape:
assignee: nobody → radix
status: New → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package landscape-client - 1.0.29-0ubuntu0.9.04.0

---------------
landscape-client (1.0.29-0ubuntu0.9.04.0) jaunty; urgency=low

  * New upstream bugfix release (LP: #358744)
    - Add a timeout to HTTP operations to avoid hanging (LP: #349737)
    - Clean up environment variables on startup to avoid propagating
      variables that will corrupt package installation (LP: #348681)
    - Clean up FDs on startup for the same reason (LP: #352458)
    - Catch and handle certain errors from smart (such as invalid package
      data) to avoid "stuck" Landscape activities (LP: #268745)
    - Don't print warnings meant for developers to the console (LP: #336669)

 -- Christopher Armstrong <email address hidden> Thu, 09 Apr 2009 17:09:50 -0400

Changed in landscape-client (Ubuntu):
status: New → Fix Released
Changed in landscape:
status: Fix Committed → Fix Released
Changed in landscape-client:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers