zhli1142015 commented on a change in pull request #28769:
URL: https://github.com/apache/spark/pull/28769#discussion_r439218088
##########
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, so why track DBIterator separately?
> If an iterator leaks, previously, it might be closed by finalize().
If an iterator leaks (i mean after db is closed), it would not be closed any
more. Here is the cause: If level db is closed, access to related JNI handle (
calling `DBIterator.close()` here ) would trigger JVM crashes. This is also
the reason that we need check if db is closed in `LevelDBcloseIterator`.
This means if db is closed, then `LevelDBIterator.finalize()` will not close
`DBIterator`. `DBIterator` and related JNI resource will be leaked till
restarting.
Yes, `DBIterator` are held, if DB is opend, `LevelDBIterator.finalize()` or
`LevelDBIterator.close` would close and release them. otherwise they would be
closed in `LevelDB.close`. This makes sure they can be closed and released in
time.
> If you hold a strong ref to all iterators, they can't be GCed. A soft
reference would allow it. If the reference is null, that's fine, it means it
was GCed and thus finalized and closed. Weak refs won't work here, yes.
About soft reference, I think it has same issue with weak reference: it's
value ( `get()` ) is marked as null before `finalize` get executed, this
behavior is same with weak reference. maybe not too often, but it's still has
chance encounter the race condition i mentioned above (iterator is not closed
in DB.close() and iterator.finalize()).
----------------------------------------------------------------
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]
