Code review comment for lp:~james-w/aptdaemon/fix-455861

Revision history for this message
Sebastian Heinlein (glatzor) wrote :

Actually you don't need to introduce a lock here, since the event sources run in the same main loop.

I think that the root of the problem is iterating on the main loop after closing the socket, which will result in processing already queued io events which will fail.

Furthermore I increased the priority of the hangup callback, so that it is preferred to copy_stdout.

See bzr diff -r 340..343

=== modified file 'aptdaemon/debconf.py'
--- aptdaemon/debconf.py 2009-10-02 11:27:03 +0000
+++ aptdaemon/debconf.py 2010-03-17 05:59:24 +0000
@@ -89,21 +89,12 @@
         """Stop listening on the socket."""
         logging.debug("debconf.stop()")
         self.socket.close()
- # ensure outstanding gio messages are processed
- context = glib.main_context_default()
- while context.pending():
- context.iteration()
         gobject.source_remove(self._listener_id)
         self._listener_id = None

     def _accept_connection(self, source, condition):
         """Callback for new connections of the passthrough frontend."""
         log.debug("New passthrough connection")
- # ensure outstanding gio messages are processed (to ensure
- # that _hangup was run on the previous connection, see LP: #432607)
- context = glib.main_context_default()
- while context.pending():
- context.iteration()
         if self._active_conn:
             raise IOError, "Multiple debconf connections not supported"
         conn, addr = source.accept()
@@ -112,15 +103,18 @@
                                        stdin=subprocess.PIPE,
                                        stdout=subprocess.PIPE,
                                        env=self._get_debconf_env())
- w = gobject.io_add_watch(conn, gobject.IO_IN|gobject.IO_HUP|gobject.IO_ERR,
- self._copy_conn, self.helper.stdin)
- self._watch_ids.append(w)
- w= gobject.io_add_watch(self.helper.stdout, gobject.IO_IN,
- self._copy_stdout, conn)
- self._watch_ids.append(w)
- w = gobject.io_add_watch(self.helper.stdout, gobject.IO_HUP|gobject.IO_ERR,
- self._hangup)
- self._watch_ids.append(w)
+ wid = gobject.io_add_watch(conn,
+ gobject.IO_IN|gobject.IO_HUP|gobject.IO_ERR,
+ self._copy_conn, self.helper.stdin)
+ self._watch_ids.append(wid)
+ wid = gobject.io_add_watch(self.helper.stdout, gobject.IO_IN,
+ self._copy_stdout, conn)
+ self._watch_ids.append(wid)
+ wid = gobject.io_add_watch(self.helper.stdout,
+ gobject.IO_HUP|gobject.IO_ERR,
+ self._hangup,
+ priority=gobject.PRIORITY_HIGH)
+ self._watch_ids.append(wid)
         return True

« Back to merge proposal