In my work for DRILL-1942, I discovered that one possible source of memory
leak is the use of preallocation (See
BufferAllocator.getNewPreAllocator()). If you pre-allocate, and then
something happens (which results in throwing an exception) before you claim
the pre-allocated space, that space (at least the accounting for it) is
lost to the allocation system forever.

As part of my fix, I'm making The PreAllocator an AutoCloseable. I choose
to do this because now the compiler will tell me if I'm not closing
PreAllocators (wherein I check to see if they're claimed, or if I should
return the requested space back to the allocator).

This is an example of letting the compiler (or other JVM ecosystem tools)
help you.

One minor snag here is that the code base has a lot of warnings in it. I've
been cleaning up the warnings in files I've been working in. (It'd be great
if we all started doing that.) Once I did that, the compiler warned me
about "leaked" preallocations.

It turns out that some of them were in various record batch handlers. When
I looked at these, I discovered that they already have an abstract method
called cleanup() that the user is required to call once they are done, in
order to clean up any off-heap allocated objects. This was a lost
opportunity: this interface should have extended AutoCloseable for the same
reason I used it. The compiler can help you find the places where you
forgot to call cleanup(). In my tree, I've made this change, and renamed
cleanup() to close() in order to take advantage of this.

One thing to think about when doing this is whether Closeable or
AutoCloseable is more appropriate. Note that Closeable.close() throws
IOException, but AutoCloseable.close() throws Exception, which is more
general. In my case, because it's about memory allocation, IOException is
not appropriate, so I used AutoCloseable.

Reply via email to