← Back to branch summary

~launchpad-pqm/launchpad/devel

« back to all changes in this revision

Viewing changes to lib/lp/codehosting/sshserver/session.py

  • Committer: Launchpad Patch Queue Manager
  • Date: 2011-02-19 19:05:38 UTC
  • mfrom: (12344.4.2 twisted-close-on-failure)
  • Revision ID: launchpad@pqm.canonical.com-20110219190538-3fwersl9zhga0t0y
[r=bac, benji][bug=717345][no-qa] If we fail to open all handles,
        close the ones we did succeed in opening.

Show diffs side-by-side

added added

removed removed

Lines of Context:
lib/lp/codehosting/sshserver/session.py
11
11
import os
12
12
import signal
13
13
import socket
 
14
import sys
14
15
import urlparse
15
16
 
16
17
from zope.event import notify
21
22
    interfaces,
22
23
    process,
23
24
    )
24
 
from twisted.python import log
 
25
from twisted.python import log, failure
 
26
from twisted.internet.main import CONNECTION_LOST, CONNECTION_DONE
25
27
 
26
28
from canonical.config import config
27
29
from lp.codehosting import get_bzr_path
129
131
            log.err('Connection failed: %s' % (e,))
130
132
            raise
131
133
        if response.startswith("FAILURE"):
 
134
            client_sock.close()
132
135
            raise RuntimeError('Failed to send message: %r' % (response,))
133
136
        return response, client_sock
134
137
 
154
157
        message.append('end\n')
155
158
        message = ''.join(message)
156
159
        response, sock = self._sendMessageToService(message)
157
 
        if response.startswith('FAILURE'):
158
 
            # TODO: Is there a better error to raise?
159
 
            raise RuntimeError("Failed while sending message to forking "
160
 
                "service. message: %r, failure: %r"
161
 
                % (message, response))
162
 
        ok, pid, path, tail = response.split('\n')
163
 
        assert ok == 'ok'
164
 
        assert tail == ''
165
 
        pid = int(pid)
166
 
        log.msg('Forking returned pid: %d, path: %s' % (pid, path))
 
160
        try:
 
161
            ok, pid, path, tail = response.split('\n')
 
162
            assert ok == 'ok'
 
163
            assert tail == ''
 
164
            pid = int(pid)
 
165
            log.msg('Forking returned pid: %d, path: %s' % (pid, path))
 
166
        except:
 
167
            sock.close()
 
168
            raise
167
169
        return pid, path, sock
168
170
 
 
171
    def _openHandleFailures(self, call_on_failure, path, flags, proc_class,
 
172
                            reactor, child_fd):
 
173
        """Open the given path, adding a cleanup as appropriate.
 
174
 
 
175
        :param call_on_failure: A list holding (callback, args) tuples. We will
 
176
            append new entries for things that we open
 
177
        :param path: The path to open
 
178
        :param flags: Flags to pass to os.open
 
179
        :param proc_class: The ProcessWriter/ProcessReader class to wrap this
 
180
            connection.
 
181
        :param reactor: The Twisted reactor we are connecting to.
 
182
        :param child_fd: The child file descriptor number passed to proc_class
 
183
        """
 
184
        fd = os.open(path, flags)
 
185
        call_on_failure.append((os.close, fd))
 
186
        p = proc_class(reactor, self, child_fd, fd)
 
187
        # Now that p has been created, it will close fd for us. So switch the
 
188
        # cleanup to calling p.connectionLost()
 
189
        call_on_failure[-1] = (p.connectionLost, (None,))
 
190
        self.pipes[child_fd] = p
 
191
 
169
192
    def _connectSpawnToReactor(self, reactor):
 
193
        self._exiter = _WaitForExit(reactor, self, self.process_sock)
 
194
        call_on_failure = [(self._exiter.connectionLost, (None,))]
170
195
        stdin_path = os.path.join(self._fifo_path, 'stdin')
171
196
        stdout_path = os.path.join(self._fifo_path, 'stdout')
172
197
        stderr_path = os.path.join(self._fifo_path, 'stderr')
173
 
        child_stdin_fd = os.open(stdin_path, os.O_WRONLY)
174
 
        self.pipes[0] = process.ProcessWriter(reactor, self, 0,
175
 
                                              child_stdin_fd)
176
 
        child_stdout_fd = os.open(stdout_path, os.O_RDONLY)
177
 
        # forceReadHack=True ? Used in process.py doesn't seem to be needed
178
 
        # here
179
 
        self.pipes[1] = process.ProcessReader(reactor, self, 1,
180
 
                                              child_stdout_fd)
181
 
        child_stderr_fd = os.open(stderr_path, os.O_RDONLY)
182
 
        self.pipes[2] = process.ProcessReader(reactor, self, 2,
183
 
                                              child_stderr_fd)
184
 
        # Note: _exiter forms a GC cycle, since it points to us, and we hold a
185
 
        # reference to it
186
 
        self._exiter = _WaitForExit(reactor, self, self.process_sock)
 
198
        try:
 
199
            self._openHandleFailures(call_on_failure, stdin_path, os.O_WRONLY,
 
200
                process.ProcessWriter, reactor, 0)
 
201
            self._openHandleFailures(call_on_failure, stdout_path, os.O_RDONLY,
 
202
                process.ProcessReader, reactor, 1)
 
203
            self._openHandleFailures(call_on_failure, stderr_path, os.O_RDONLY,
 
204
                process.ProcessReader, reactor, 2)
 
205
        except:
 
206
            exc_class, exc_value, exc_tb = sys.exc_info()
 
207
            for func, args in call_on_failure:
 
208
                try:
 
209
                    func(*args)
 
210
                except:
 
211
                    # Just log any exceptions at this point. This makes sure
 
212
                    # all cleanups get called so we don't get leaks. We know
 
213
                    # there is an active exception, or we wouldn't be here.
 
214
                    log.err()
 
215
            raise exc_class, exc_value, exc_tb
187
216
        self.pipes['exit'] = self._exiter
188
217
 
189
218
    def _getReason(self, status):
248
277
    # Implemented because ProcessWriter/ProcessReader want to call it
249
278
    # Copied from twisted.internet.Process
250
279
    def childConnectionLost(self, childFD, reason):
251
 
        close = getattr(self.pipes[childFD], 'close', None)
252
 
        if close is not None:
253
 
            close()
254
 
        else:
255
 
            os.close(self.pipes[childFD].fileno())
256
 
        del self.pipes[childFD]
 
280
        pipe = self.pipes.get(childFD)
 
281
        if pipe is not None:
 
282
            close = getattr(pipe, 'close', None)
 
283
            if close is not None:
 
284
                close()
 
285
            else:
 
286
                os.close(self.pipes[childFD].fileno())
 
287
            del self.pipes[childFD]
257
288
        try:
258
289
            self.proto.childConnectionLost(childFD)
259
290
        except: