Exception when saving

[Austin Schuh]

I'm trying to backup my desktop and getting a weird exception. I'm running:

bup index --exclude /sys --exclude /var/lib/docker/overlay2 --exclude
/run --exclude /dev --exclude /proc --exclude /tmp --exclude
/root/.bup --exclude /var/cache/apt --exclude /media /
bup save -r synology-bup: -n aschuh-3950x /

Reading index: 142349, done.
[Errno 2] No such file or directory:
'/home/austin/.config/google-chrome/Profile 1/Local
Storage/leveldb/867972.ldb'
[Errno 2] No such file or directory:
'/home/austin/.config/google-chrome/Profile 1/Local
Storage/leveldb/867971.ldb'
[Errno 2] No such file or directory:
'/home/austin/.config/google-chrome/Profile 1/Local
Storage/leveldb/867969.log'
[Errno 2] No such file or directory:
'/home/austin/.config/google-chrome/Default/IndexedDB/https_docs.google.com_0.indexeddb.blob/4/13/313f5'
[Errno 2] No such file or directory:
'/home/austin/.config/google-chrome/Default/IndexedDB/https_calendar.google.com_0.indexeddb.leveldb/577320.log'
Receiving index from server: 5601072/5601072, done.
Saving: 99.66% (2881936/2891803k, 142349/142349 files), done.
Receiving index from server: 628048/628048, done.
WARNING: 5 errors encountered while saving.
Exception ignored in: <function PackWriter_Remote.__del__ at 0x7f66cb736980>
Traceback (most recent call last):
File "/usr/lib/bup/bup/client.py", line 571, in __del__
TypeError: super() argument 1 must be a type, not NoneType

Any ideas what could be throwing that exception? I'm running the
Debian stable versions:

root[2084] aschuh-3950x ~/bin
# bup version
0.33.2
root[2085] aschuh-3950x ~/bin
# ssh synology-bup bup version
0.33.2

# dpkg -l | grep bup
ii bup 0.33.2-1~deb12u1
amd64 highly efficient file backup
system based on git

Thanks!
Austin

[Rob Browning]

Austin Schuh <austin...@gmail.com> writes:

> Exception ignored in: <function PackWriter_Remote.__del__ at 0x7f66cb736980>
> Traceback (most recent call last):
> File "/usr/lib/bup/bup/client.py", line 571, in __del__
> TypeError: super() argument 1 must be a type, not NoneType
>
> Any ideas what could be throwing that exception?

So, I suspect this is just code that's intended to detect trouble,
causing trouble.

If it's easy for you to reproduce, and if you want to, I'd be curious if
changing the code in client.py (around line 571) from this:

super(PackWriter_Remote, self).__del__()

to this:

super().__del__()

makes any difference. Though I suspect it won't, so don't worry about
it too much.

I've been a little hesitant about some of our __del__ methods, since I
know the environment when they run is variously precarious:

https://docs.python.org/3/reference/datamodel.html#object.__del__

I'm guessing that in this case the global PackWriter_Remote binding has
been cleared, and it's possible that super() won't have that problem. I
think there might also be an alternate formulation we could try if we
really wanted to keep this, i.e.

super(self.__class__, self).__del__()


In the end, we're just using these for (invariant) sanity checking,
e.g. to try to make sure bup always closes files/mmaps/... properly.

The superclass __del__ just does something similar, i.e:

assert self.closed

Oh, and did this actually cause bup to exit with a non-zero status if
you know? Offhand I vaguely recalled that maybe it didn't, i.e. from
__del__ it may just complain to stderr and move on.

Thanks
--
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4

[Rob Browning]

Rob Browning <r...@defaultvalue.org> writes:

> super().__del__()
>
> makes any difference. Though I suspect it won't, so don't worry about
> it too much.

So that does fix it; tested via:

class Foo:
def __del__(self): pass

class Bar(Foo):
def __del__(self):
super().__del__()

x = Bar()
Bar = None

Run via "python 3 test.py", it won't crash, but it will if you change
the super() invocation back to the style bup's using.

I'll plan to change bup to use that in 0.33.x and main where it's
relevant.

Thanks again

[Rob Browning]

Although the two should normally be equivalent, they're not if the
class binding is no longer available or has changed. The latter can
be the case when called from __del__ (the value may be None).

Thanks to Austin Schuh for reporting the issue.

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---

Pushed to 0.33.x and pending for main.

lib/bup/client.py | 4 ++--
lib/bup/cmd/web.py | 2 +-
lib/bup/git.py | 4 ++--
lib/bup/io.py | 6 +++---
test/ext/conftest.py | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/bup/client.py b/lib/bup/client.py
index c5751eea9..2cf157e62 100644
--- a/lib/bup/client.py
+++ b/lib/bup/client.py
@@ -561,12 +561,12 @@ class PackWriter_Remote(git.PackWriter):
self.remote_closed = True
id = self._end()
self.file = None
- super(PackWriter_Remote, self).close()
+ super().close()
return id

def __del__(self):
assert self.remote_closed
- super(PackWriter_Remote, self).__del__()
+ super().__del__()

def abort(self):
raise ClientError("don't know how to abort remote pack writing")
diff --git a/lib/bup/cmd/web.py b/lib/bup/cmd/web.py
index c609d872e..8078f934b 100644
--- a/lib/bup/cmd/web.py
+++ b/lib/bup/cmd/web.py
@@ -116,7 +116,7 @@ class BupRequestHandler(tornado.web.RequestHandler):
def decode_argument(self, value, name=None):
if name == 'path':
return value
- return super(BupRequestHandler, self).decode_argument(value, name)
+ return super().decode_argument(value, name)

def get(self, path):
return self._process_request(path)
diff --git a/lib/bup/git.py b/lib/bup/git.py
index ba34fdaaf..90a0d6347 100644
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -410,7 +410,7 @@ class PackIdx(object):
class PackIdxV1(PackIdx):
"""Object representation of a Git pack index (version 1) file."""
def __init__(self, filename, f):
- super(PackIdxV1, self).__init__()
+ super().__init__()
self.closed = False
self.name = filename
self.idxnames = [self.name]
@@ -464,7 +464,7 @@ class PackIdxV1(PackIdx):
class PackIdxV2(PackIdx):
"""Object representation of a Git pack index (version 2) file."""
def __init__(self, filename, f):
- super(PackIdxV2, self).__init__()
+ super().__init__()
self.closed = False
self.name = filename
self.idxnames = [self.name]
diff --git a/lib/bup/io.py b/lib/bup/io.py
index e2fd410b3..3fcca1cbb 100644
--- a/lib/bup/io.py
+++ b/lib/bup/io.py
@@ -37,17 +37,17 @@ class mmap(py_mmap.mmap):

def close(self):
self._bup_closed = True
- super(mmap, self).close()
+ super().close()

if hasattr(py_mmap.mmap, '__enter__'):
def __enter__(self):
- super(mmap, self).__enter__()
+ super().__enter__()
return self
def __exit__(self, type, value, traceback):
# Don't call self.close() when the parent has its own __exit__;
# defer to it.
self._bup_closed = True
- result = super(mmap, self).__exit__(type, value, traceback)
+ result = super().__exit__(type, value, traceback)
return result
else:
def __enter__(self):
diff --git a/test/ext/conftest.py b/test/ext/conftest.py
index 287a68d02..f2845d642 100644
--- a/test/ext/conftest.py
+++ b/test/ext/conftest.py
@@ -14,7 +14,7 @@ use_node_path = hasattr(pytest, 'version_tuple')

class BupSubprocFailure(Exception):
def __init__(self, msg, cmd, status, failures):
- super(BupSubprocFailure, self).__init__(msg)
+ super().__init__(msg)
self.cmd = cmd
self.status = status
self.failures = failures
@@ -22,7 +22,7 @@ class BupSubprocFailure(Exception):
class BupSubprocTestRunner(pytest.Item):

def __init__(self, name, parent):
- super(BupSubprocTestRunner, self).__init__(name, parent)
+ super().__init__(name, parent)

def runtest(self):
cmd = str(self.fspath)
--
2.39.2

[Austin Schuh]

Hi Rob,

On Sat, Oct 7, 2023 at 11:12 AM Rob Browning <r...@defaultvalue.org> wrote:
>
> Rob Browning <r...@defaultvalue.org> writes:
>
> > super().__del__()
> >
> > makes any difference. Though I suspect it won't, so don't worry about
> > it too much.
>
> So that does fix it; tested via:
>
> class Foo:
> def __del__(self): pass
>
> class Bar(Foo):
> def __del__(self):
> super().__del__()
>
> x = Bar()
> Bar = None

That fixes it for me, thanks! I attempted to send out a patch for the
one in client.py with the change I was able to test locally, but it
appears that it didn't go through to the mailing list.

Austin

[Austin Schuh]

On Sat, Oct 7, 2023 at 11:40 AM Rob Browning <r...@defaultvalue.org> wrote:
>
> Although the two should normally be equivalent, they're not if the
> class binding is no longer available or has changed. The latter can
> be the case when called from __del__ (the value may be None).
>
> Thanks to Austin Schuh for reporting the issue.
>
> Signed-off-by: Rob Browning <r...@defaultvalue.org>
> Tested-by: Rob Browning <r...@defaultvalue.org>

Looks good to me, thanks!

@Robert Edmonds, this patch should fix
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053547. I've been
hand-patching my bookworm systems with part of Rob's fix for the time
being.

Austin

[Rob Browning]

Rob Browning <r...@defaultvalue.org> writes:

> Although the two should normally be equivalent, they're not if the
> class binding is no longer available or has changed. The latter can
> be the case when called from __del__ (the value may be None).
>
> Thanks to Austin Schuh for reporting the issue.
>
> Signed-off-by: Rob Browning <r...@defaultvalue.org>
> Tested-by: Rob Browning <r...@defaultvalue.org>
> ---
>
> Pushed to 0.33.x and pending for main.

Pushed to main.