zhli1142015 commented on pull request #28769: URL: https://github.com/apache/spark/pull/28769#issuecomment-641689643
> I'm not suggesting leaks are OK? I'm suggesting figuring out and fixing the leaks directly, not cleaning them up later indirectly if possible. Being `Closeable` isn't the issue per se, nor `.asScala`, but getting them closed in all cases may be tricky. But I don't know if we'll find and fix all of them; we can fix one at least, even if we also have to resort to the cleanup method as well in this PR to catch the other unknown paths. @srowen , i have different opinion about cause of leaking here. For most cases, if `onUIDetached`(calls `loadedUI.ui.store.close()`) is not triggered, iterator's JNI resource would be cleaned by try-with-resource or GC finally, these would not become leaking resources. i think issue here is we introduce close method in LevelDb class. This method should be responsible for cleaning all native resources, or at least not prevent iterator clean after it. But both of them are not truth. Considering `iterator.close()` is not allowed to be called after db is closed, so i proposed this PR to solve the issue: clean all native resources (include iterator.close()) in it. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
