zhli1142015 commented on a change in pull request #28769:
URL: https://github.com/apache/spark/pull/28769#discussion_r439180627



##########
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:
       yes, you are right, this is a little complex. the issue here is if 
tracking `LevelDBIterator`, and some callers don't explicitly close it, 
`LevelDBIterator.finalize` can't be executed as `LevelDBIterator` are 
referenced in `LevelDB`. There may be the case elements in `iteratorTracker` 
may always increase but not get released till DB is closed. i think this would 
introduce memory leak in this case.
   If all callers explicitly close `LevelDBIterator`, then we can track 
`LevelDBIterator` here.




----------------------------------------------------------------
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]

Reply via email to