thisisnic commented on PR #51288: URL: https://github.com/apache/arrow/pull/51288#issuecomment-5621326880
@jonkeane - I had Claude walk me though this but the explanation below is all mine; tried to give a more understandable explanation of what's going on here. I feel I should probably just ditch the test instead of including one which calls `gctorture()` but figured leaving it here for a moment while we discuss this gives us a bit more context here. ## Bug context My understanding of this is that this is a bug which could trigger in specific occasions, where we have a string column that's been read in from a Parquet file or other Arrow -> R conversion path, *and* is being represented by altrep. When Arrow creates R strings we can end up with a situation where they're left floating around in memory with no pointer, and the garbage collector gets rid of it even though R still needs it. This only affects unmaterialised values. It happens in extremely specific circumstances: when an R function accesses an element of this column and then allocates memory before it has finished using the value. The allocation triggers the garbage collector, which frees the string, and we end up with a segfault or incorrect value. ## This solution ### Code We basically create a cache that is attached to the ALTREP vector which lists the pointers of all the strings which have been accessed. We only add stuff to the cache when it's accessed. We do this lazily in chunks like the vroom PR which is linked to by the issue author. :robot: told me to add: "When the column is later materialised, cached strings are copied into the new vector instead of converted again so they stay alive after the cache is dropped" ### Tests The test here uses `gctorture()` which makes the garbage collector run on every single allocation, otherwise we wouldn't see the error (n.b. the garbage collector only usually runs when when memory crosses a threshold). I ran the test without the fix and then with the fix, and got a segfault and then no segfault. I had a minor concern is that this is a bug that only shows up in extremely specific circumstances, which may be rare, but the fix increases memory footprint for elements that have been accessed. But no worse than not using altrep at all. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
