From aeffdb776ded3ae2a95f1a216fdcbcb7dc2dc8f2 Mon Sep 17 00:00:00 2001 From: Rob Browning Date: Sat, 1 Jan 2022 11:13:41 -0600 Subject: [PATCH] Client: make close durable; clean up partitial initializations Always call close after any __init__ errors, and make sure close always tries to clean up everything that's been initialized so far. This fixes a __del__ complaint exposed when using the mmap wrapper in py3. Signed-off-by: Rob Browning Tested-by: Rob Browning --- lib/bup/client.py | 151 ++++++++++++++++++++++++++-------------------- 1 file changed, 84 insertions(+), 67 deletions(-) diff --git a/lib/bup/client.py b/lib/bup/client.py index c4f2bff..8af357f 100644 --- a/lib/bup/client.py +++ b/lib/bup/client.py @@ -72,78 +72,95 @@ class Client: self.closed = False self._busy = self.conn = None self.sock = self.p = self.pout = self.pin = None - is_reverse = environ.get(b'BUP_SERVER_REVERSE') - if is_reverse: - assert(not remote) - remote = b'%s:' % is_reverse - (self.protocol, self.host, self.port, self.dir) = parse_remote(remote) - # The b'None' here matches python2's behavior of b'%s' % None == 'None', - # python3 will (as of version 3.7.5) do the same for str ('%s' % None), - # but crashes instead when doing b'%s' % None. - cachehost = b'None' if self.host is None else self.host - cachedir = b'None' if self.dir is None else self.dir - self.cachedir = git.repo(b'index-cache/%s' - % re.sub(br'[^@\w]', - b'_', - b'%s:%s' % (cachehost, cachedir))) - if is_reverse: - self.pout = os.fdopen(3, 'rb') - self.pin = os.fdopen(4, 'wb') - self.conn = Conn(self.pout, self.pin) - else: - if self.protocol in (b'ssh', b'file'): - try: - # FIXME: ssh and file shouldn't use the same module - self.p = ssh.connect(self.host, self.port, b'server') - self.pout = self.p.stdout - self.pin = self.p.stdin - self.conn = Conn(self.pout, self.pin) - except OSError as e: - reraise(ClientError('connect: %s' % e)) - elif self.protocol == b'bup': - self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - self.sock.connect((self.host, - 1982 if self.port is None else int(self.port))) - self.sockw = self.sock.makefile('wb') - self.conn = DemuxConn(self.sock.fileno(), self.sockw) - self._available_commands = self._get_available_commands() - self._require_command(b'init-dir') - self._require_command(b'set-dir') - if self.dir: - self.dir = re.sub(br'[\r\n]', ' ', self.dir) - if create: - self.conn.write(b'init-dir %s\n' % self.dir) + try: + is_reverse = environ.get(b'BUP_SERVER_REVERSE') + if is_reverse: + assert(not remote) + remote = b'%s:' % is_reverse + (self.protocol, self.host, self.port, self.dir) = parse_remote(remote) + # The b'None' here matches python2's behavior of b'%s' % None == 'None', + # python3 will (as of version 3.7.5) do the same for str ('%s' % None), + # but crashes instead when doing b'%s' % None. + cachehost = b'None' if self.host is None else self.host + cachedir = b'None' if self.dir is None else self.dir + self.cachedir = git.repo(b'index-cache/%s' + % re.sub(br'[^@\w]', + b'_', + b'%s:%s' % (cachehost, cachedir))) + if is_reverse: + self.pout = os.fdopen(3, 'rb') + self.pin = os.fdopen(4, 'wb') + self.conn = Conn(self.pout, self.pin) else: - self.conn.write(b'set-dir %s\n' % self.dir) - try: + if self.protocol in (b'ssh', b'file'): + try: + # FIXME: ssh and file shouldn't use the same module + self.p = ssh.connect(self.host, self.port, b'server') + self.pout = self.p.stdout + self.pin = self.p.stdin + self.conn = Conn(self.pout, self.pin) + except OSError as e: + reraise(ClientError('connect: %s' % e)) + elif self.protocol == b'bup': + self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self.sock.connect((self.host, + 1982 if self.port is None else int(self.port))) + self.sockw = self.sock.makefile('wb') + self.conn = DemuxConn(self.sock.fileno(), self.sockw) + self._available_commands = self._get_available_commands() + self._require_command(b'init-dir') + self._require_command(b'set-dir') + if self.dir: + self.dir = re.sub(br'[\r\n]', ' ', self.dir) + if create: + self.conn.write(b'init-dir %s\n' % self.dir) + else: + self.conn.write(b'set-dir %s\n' % self.dir) self.check_ok() - except BaseException as ex: - with pending_raise(ex): - self.close() - self.sync_indexes() + self.sync_indexes() + except BaseException as ex: + with pending_raise(ex): + self.close() def close(self): + if self.closed: + return self.closed = True - if self.conn and not self._busy: - self.conn.write(b'quit\n') - if self.pin: - self.pin.close() - if self.sock and self.sockw: - self.sockw.close() - self.sock.shutdown(socket.SHUT_WR) - if self.conn: - self.conn.close() - if self.pout: - self.pout.close() - if self.sock: - self.sock.close() - if self.p: - self.p.wait() - rv = self.p.wait() - if rv: - raise ClientError('server tunnel returned exit code %d' % rv) - self.conn = None - self.sock = self.p = self.pin = self.pout = None + try: + if self.conn and not self._busy: + self.conn.write(b'quit\n') + finally: + try: + if self.pin: + self.pin.close() + finally: + try: + self.pin = None + if self.sock and self.sockw: + self.sockw.close() + self.sock.shutdown(socket.SHUT_WR) + finally: + try: + if self.conn: + self.conn.close() + finally: + try: + self.conn = None + if self.pout: + self.pout.close() + finally: + try: + self.pout = None + if self.sock: + self.sock.close() + finally: + self.sock = None + if self.p: + self.p.wait() + rv = self.p.wait() + if rv: + raise ClientError('server tunnel returned exit code %d' % rv) + self.p = None def __del__(self): assert self.closed -- 2.39.2