0

This is a follow-up question from this one.

Specifically I'm trying to verify correct use of an API which involves calling close() on Java objects holding heavy native resources over JNI. To re-iterate from that question, this is needed in both tests and production, tests because resources leak from one testcase to another and in production because, well, native resources should just be disposed in controlled manner.

The code (all such Java objects that I want disposed would extend JPeer):

public class JPeer implements AutoCloseable {

    private final long nativeHandle_ = ....

    private final Verifier closeVerifier_;
    private static final Cleaner cleaner_ = Cleaner.create();

    protected JPeer() {
        closeVerifier_ = new Verifier();
        cleaner_.register(this, closeVerifier_);
    }

    @SuppressWarnings("unchecked")
    @Override
    public final void close() {
        if (closed())
            throw new AssertionError("JPeer already closed");

        ..... // doing dispose over JNI

        closeVerifier_.markDone();
    }



//    @SuppressWarnings("deprecation")
//    @Override
//    protected final void finalize() {
//        if (!closed()) {
//            // due to threading considerations with JNI, we shouldn't destroy here, we rely on user to destroy from construction thread
//                // TODO finalize() never called, how to assert closure?
//            throw new AssertionError("JPeer was not closed, native object leaking");
//        }
//    }

    private static final class Verifier implements Runnable {

        volatile boolean done_ = false;

        public void markDone() {
            done_ = true;
        }

        @Override
        public void run() {
            if (!done_) {
                // due to threading considerations with JNI, we shouldn't destroy here, we rely on user to destroy from construction thread
                System.err.println("Verifier: JPeer was not closed, native object leaking");
                throw new AssertionError("Verifier: JPeer was not closed, native object leaking");
            }
            else {
                System.out.println("Verifier: JPeer was closed");
            }
        }
    }
}

Why is the cleaner action not invoked? I see neither "JPeer was closed" nor "JPeer not closed" in the console. It is a problem with not being able to print something from a cleaner action? (I know exceptions thrown from there are ignored so the AssertionError is useless) If so, how can I notify the outcome of my verification?

haelix
  • 4,245
  • 4
  • 34
  • 56
  • Do you invoke either your `cleaner_` or `closeVerifier_.run`? I just see you calling `markDone`. – Taylor Feb 12 '19 at 14:04
  • @Taylor sorry the c-tor was missing, added now – haelix Feb 12 '19 at 14:06
  • Wouldn't you need to invoke(i.e. call) them from the `close()` method? – Taylor Feb 12 '19 at 14:18
  • @Taylor no, what would be the point then. Doc says _Cleaning actions are registered to run after the cleaner is notified that the object has become phantom reachable_ – haelix Feb 12 '19 at 17:28
  • Hmm, sorry, not familiar with `Cleaner`, I had assumed it was a custom class. In reading the docs, `Cleaner` will only run when your object is phantom reachable, so is it long-lived? Are you holding a reference to it? – Taylor Feb 12 '19 at 17:36
  • @Taylor it's long-lived. I tried setting all references to null. – haelix Feb 12 '19 at 17:53
  • Best advice I can give is to check all references to it, and you may need a heap dump to see who's holding a ref. – Taylor Feb 12 '19 at 18:23
  • It requires an actual garbage collection to identify phantom reachable objects. Unless you trigger it manually (which is not guaranteed to actually do anything), it is only driven by memory needs. As long as there is enough memory to operate, the JVM can run without a garbage collection for an arbitrary long time. As a side note, for production code, the `close()` method should unregister the `Cleaner`, so well behaving program do not have to pay its costs. – Holger Feb 26 '19 at 13:07
  • @Holger thanks - for the record, after trying 2 techniques for cleaning up things - (1) finalisers and (2) Cleaners - both of which are invoked _sometimes_, I had good success with `Runtime.getRuntime().addShutdownHook()`. Inside the hook I only check that an object was disposed correctly (by the user), I don't do the disposal itself. – haelix Feb 26 '19 at 13:12
  • Checking the objects in a shutdown hook implies that you have access to the existing instances, which would prevent their garbage collection. A reasonable strategy would again be to remove the objects in `close()`, so the shutdown hook does not need to check; the remaining objects are not closed. Being invoked *sometimes* is exactly the expected behavior for finalizers and cleaners, as said, because they depend on something that is only driven by memory needs. The question is, how important is reporting the error; a lot of APIs and frameworks do error checking on a *best-effort* basis only. – Holger Feb 26 '19 at 13:23
  • _Checking the objects in a shutdown hook implies that you have access_ - It does not imply. Checking can be done by simply making the shutdown hook hold a boolean flag that is initially set, and cleared by `MyObject.close()`- if the flag is still set that means close was not called, and off I go with an error logline. How important - arguably not the most important thing to check this, but in some situations it may be - in my case, proper shutdown is the only way for an app to be sure all data was sent to a peer, because there is a termination protocol. – haelix Feb 27 '19 at 17:22

0 Answers0