zhli1142015 commented on a change in pull request #28769:
URL: https://github.com/apache/spark/pull/28769#discussion_r439183847
##########
File path:
common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDB.java
##########
@@ -247,17 +266,28 @@ public void close() throws IOException {
}
}
+ public boolean isClosed() {
+ return this._db.get() == null;
+ }
+
/**
* Closes the given iterator if the DB is still open. Trying to close a JNI
LevelDB handle
* with a closed DB can cause JVM crashes, so this ensures that situation
does not happen.
*/
- void closeIterator(LevelDBIterator<?> it) throws IOException {
+ public void closeIterator(DBIterator it) throws IOException {
synchronized (this._db) {
DB _db = this._db.get();
if (_db != null) {
it.close();
}
}
+ iteratorTracker.remove(it);
+ }
+
+ public DBIterator createIterator() {
Review comment:
`DBIterator` is `LevelDBIterator`'s property, i think it would not block
`LevelDBIterator` being collected by GC. Then `LevelDBIterator.finalize` would
close `DBIterator` and remove it from `iteratorTracker`.
I tried soft reference and weak reference, these two would not help with
leaking issue: they would be marked as null, when finalize ready. so here maybe
the case GC and DB.close are executed same time. If the execution sequence is
1) weak reference is marked null, 2) `DB.close` then 3)
`LevelDBIterator.finalize`. JNI resource still is leaked (`ReferenceQueue` also
can't help as there is no guarantee for timing). UT i added can reproduce this.
----------------------------------------------------------------
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]
