flambeau icon indicating copy to clipboard operation
flambeau copied to clipboard

To `lent` or not to `lent`

Open HugoGranstrom opened this issue 3 years ago • 4 comments

Right now we use lent UncheckedArray instead of ptr UncheckedArray in some places. And at the moment lent is handled behind the scenes with pointers but from this conversation with @Clyybber it isn't guaranteed to stay that way in the future. Should we just change all functions returning lent UncheckedArray to ptr UncheckedArray to be sure it won't break in the future?

HugoGranstrom avatar Apr 30 '21 16:04 HugoGranstrom

The advantage of lent is that it avoids people escaping with a ptr and then introducing use-after-free bugs which are hard to debug.

What we can do is write a test that ensures we don't miss the transition and then use lent ptr UncheckedArray.

mratsim avatar May 03 '21 15:05 mratsim

Ok that's a good point 👍

What does it mean to lend a ptr though? Doesn't it just mean that the pointer itself won't be released but whatever it points to could have been released? It's better than nothing regardless 😄

One other problem, if we have a lent Uncheckedarray and want to pass it to a proc but the proc wants a ptr Uncheckedarray. Is there a way to keep the lent part? Ie is there an alternative to passing in the unsafeAddr of the Uncheckedarray?

HugoGranstrom avatar May 04 '21 11:05 HugoGranstrom

This just regards the ArrayRef wrapped type since for Tensor & RawTensor data_ptr already returns a ptr UncheckedArray[T], right ?

I think I would go with a pt UncheckedArray for internal use.

For public API, we should try to mostly expose openArray / seq and do the conversion internally.

For the few that may remains, it will mainly involve seq/openArray with low number of element to ArrayRef[T] to pass as parameters so even exposing a copyMem based API could be enough.

Clonkk avatar May 05 '21 10:05 Clonkk

Yes that seems to be the case, although it may very well be that I changed them to ptr in the past because I was frustrated with how they didn't cooperate with me 🤣

Yes, that seems like a reasonable thing to do. The user shouldn't be exposed to these things unless specifically asking for them (data_ptr etc). Openarray should be the standard type we expose to the user in my opinion 👍

HugoGranstrom avatar May 06 '21 13:05 HugoGranstrom