Hi Antoine,

> AFAIU, the problem only exists with ResizableBuffer?

I think, it comes down to the memory ownership. While Buffer apparently
never owns it's memory (based on the doc string), a MutableBuffer can. So
if you slice a MutableBuffer, and the memory gets deallocated, you've got
the same problem.

Your proposed solution seems to be a safe one, we could even use
shared_ptr::use_counts. But I'd rather invert the responsibility: a parent
buffer can do with it's memory as it pleases, the slices get a way of
checking if they are still valid. Maybe we could even include a safe cast
to void*, uint8_t* and char* for the buffers (which checks buffers
validity), so to discourage reusing of a stale data pointer (like the
raw_data* in some ArrayBuilders). So instead of memcpy(buffer.data(),
source, N) you'd just use memcpy(buffer, source, N).

So the logic would basically be: a Buffer either owns it's memory or is a
slice (view). If it's a slice, then it has a way of checking if it's still
valid (and hasn't shrunk so the view is not valid anymore). If the slice is
built with a raw pointer, then all bets are off, but we should mark such
cases as super unsafe, i.e. UnsafeBuffer.

Maybe I'm overthinking it, but I can imagine, that it'll come to bite us,
when the code base grows.


On Wed, Apr 11, 2018 at 11:19 AM, Antoine Pitrou <anto...@python.org> wrote:

> Hi Dimitri,
> Le 11/04/2018 à 09:02, Dimitri Vorona a écrit :
> > Hi everybody,
> >
> > to continue the discussion in [0]: right now this [1] can happen and the
> > sliced buffer has no way to foresee or to check against it beforehand.
> >
> > I'd suggest to create a new class SlicedBuffer, which would reference the
> > parent buffer and return it's data() pointer, insted of grabbing the
> > pointer at creation. We can the parent reference a weak pointer, so the
> > existing SliceBuffers don't prevent the deallocation (or maybe even have
> a
> > separate WeakSliceBuffer class).
> AFAIU, the problem only exists with ResizableBuffer?
> We could do the following:
> - add a new Buffer member counting the number of exports (default 0)
> - in SliceBuffer, increment the parent's number of exports
> - in ~Buffer, decrement the parent's number of exports
> - in PoolBuffer::Resize(), return an error if the buffer has any exports
> (this is how Python buffers work, for the record)
> What do you think?
> Regards
> Antoine.

Reply via email to