See there:

https://github.com/borgbackup/borg/pull/4247

I did the first changeset after seeing some strange exception popping up which it was handling another exception - which I assumed was related to memoryview.release not being called in the original code.

So it was clear to me, that we should use the CM there. So I added that (first changeset) and that made the code always fail (see first travis-ci link).

Then removed the CM again and replaced it with functionally equivalent try/finally (second changeset) - that worked.

So, the question is whether there is some issue in CPython's memoryview contextmanager code that make it fail in such a strange way.

Thanks for the report. Can you please add a simple reproducer in Python with what the test is trying to do without dependencies from the project? perhaps with a sample of relevant files used by the test in Travis.

We use "crash" for segmentation fault, but this appears to be a regular traceback that includes BufferError.

The BufferError message appears to be from mmapmodule.c.

The behavior seems to be correct to me: If there are exports, the memoryview cannot be released.

The application needs to ensure that release() is not called when
there are exports left.

working, but potentially problematic because .release is not always called (e.g. in case of an exception).

with context manager, does not work.

with try/finally: works.

hmm, issue tracker does not make it easy to see which file was uploaded for which comment, so:

a: original code, works, potentially problematic exception handling (memoryview not always being released)

b: using the memoryview context manager, fails with BufferError

c: using try/finally to make sure memoryview is released: works

I see 2 issues here:

1. I would expect that the CM approach (b) behaves equivalent to try/finally (c), but it does not and even causes an exception.

2. The traceback given in the BufferError (in (b)) is misleading, it points to the "print", but should rather point to mmap.__exit__).

Well, the problem in b) is that data[:2] creates a new memoryview,
so the underlying ManagedBufferObject now has two exports:

  - One from the context manager.

  - The second from the slice.

So memoryview.__exit__() decrements on export, but the second one
is hanging.

This actually works as expected because the ManagedBufferObject
cannot know that it could also release the slice. That's what I
meant by saying that it's the application's responsibility to
release all views that are based on the context manager's view.


One way of doing so would be this:

with open(fn, 'rb') as fd:
    with mmap.mmap(fd.fileno(), 0, access=mmap.ACCESS_READ) as mm:
        with memoryview(mm) as data:
            with data[:2] as theslice:
                print(theslice)

s/on export/one export/

Or, obviously:

with open(fn, 'rb') as fd:
    with mmap.mmap(fd.fileno(), 0, access=mmap.ACCESS_READ) as mm:
        with memoryview(mm)[:2] as data:
            print(data)

> Or, obviously:
> 
> with open(fn, 'rb') as fd:
>     with mmap.mmap(fd.fileno(), 0, access=mmap.ACCESS_READ) as mm:
>         with memoryview(mm)[:2] as data:
>             print(data)

Doesn't this rely on the immediate finalization of the unreferenced memoryview instance that creates the slice? It would be nice if memoryview supported alternate constructors memoryview(obj, stop) and memoryview(obj, start, stop[, step]).

https://github.com/borgbackup/borg/pull/4247/files

this is the current code i have for this (it is not as simple there as in the minimal code samples i attached here).

i meanwhile think i can not use a contextmanager there.

do you think this is as good as it gets for this kind of code?

if you all think this is expected behaviour for the python contextmanagers and can't be done better, this issue can be closed.

Eryk Sun:

Yes, the behavior is technically not guaranteed. I'm not sure about
memoryview(x, start, stop, step) but I'll keep it in mind.


Thomas Waldmann:
> do you think this is as good as it gets for this kind of code?

I guess so, there's a lot going on in that code fragment.