UMat.getMat leads to JVM crash while garbage collection

[Daniel-Alievsky]

There is a serious problem while using UMat and Mat in Java, namely when we call UMat.getMat method, both in JavaCPP for OpenCV 3.4.0 and for OpenCV 4.0.0. Let's suppose we have some Mat source. We convert it to UMat to perform something, and then convert result back to Mat:

final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
opencv_imgproc.GaussianBlur(uMat, uMat, KSIZE, 0.0);
final opencv_core.Mat result = uMat.getMat(opencv_core.ACCESS_RW);
result.close();
uMat.close();

This works fine: derived Mat result released 1st, uMat released 2nd. But if we release uMat 1st, and result 2nd, it can (sometimes) lead to error: Exception in thread "main" java.lang.RuntimeException: OpenCV(4.0.0) C:\projects\javacpp-presets\opencv\cppbuild\windows-x86_64-gpu\opencv-4.0.0\modules\core\src\ocl.cpp:4766: error: (-215:Assertion failed) u->refcount == 0 && "UMat deallocation error: some derived Mat is still alive" in function 'cv::ocl::OpenCLAllocator::deallocate'

But this code is absurd: it closes the result before anyone used it. Let's try to create some usual Java-style function, that does not close the result:

public opencv_core.Mat test(opencv_core.Mat source) {
    final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
    opencv_imgproc.GaussianBlur(uMat, uMat, KSIZE, 0.0);
    final opencv_core.Mat result = uMat.getMat(opencv_core.ACCESS_RW);
    return result;
}

Seems Ok. It will work... 10 times, 100 times... but sometimes, when garbage collector will start to utilize garbage, it will call uMat.close() and result.close() in ILLEGAL order, and the system will crash - exceptions while garbage collection stop JVM by default.

However, this function seems to be absolutely correct. Maximum, I can want to add uMat.close() before returning result, but it will be incorrect (illegal order!).

I found a workaround:

 public static opencv_core.Mat toMatAndClose(opencv_core.UMat u) {
    final opencv_core.Mat m = u.getMat(opencv_core.ACCESS_RW);
    try {
        return m.clone();
    } finally {
        m.close();
        u.close();
        // - this way is necessary to avoid error
        // OpenCV Error: Assertion failed (u->refcount == 0 &&
        //   "UMat deallocation error: some derived Mat is still alive")
        // while garbage collection (leading to crash of JVM)
    }
}
public opencv_core.Mat test(opencv_core.Mat source) {
    final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
    opencv_imgproc.GaussianBlur(uMat, uMat, KSIZE, 0.0);
    return toMatAndClose(uMat);
}

Every time, when I need to call UMat.getMat, now I must use the function toMatAndClose - maybe with uMat.clone, if I need to preserve the original UMat. It works, But I am not sure that there are no other troubles like this - for example, with Mat.getUMat and other functions.

Is it possible to correct this behavior of JavaCPP deallocator? Any other ideas?

Below is the full class illustration the problem and several variants of work without errors:

package com.siams.stare.extensions.javacpp.opencv;

import org.bytedeco.javacpp.opencv_core;
import org.bytedeco.javacpp.opencv_imgcodecs;
import org.bytedeco.javacpp.opencv_imgproc;

import java.io.FileNotFoundException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class SimpleUMatUsageTest {
    private static final opencv_core.Size KSIZE = new opencv_core.Size(55, 55);

    public static opencv_core.Mat toMatAndClose(opencv_core.UMat u) {
        final opencv_core.Mat m = u.getMat(opencv_core.ACCESS_RW);
        try {
            return m.clone();
        } finally {
            m.close();
            u.close();
            // - this way is necessary to avoid error
            // OpenCV Error: Assertion failed (u->refcount == 0 &&
            //   "UMat deallocation error: some derived Mat is still alive")
            // while garbage collection (leading to crash of JVM)
        }
    }

    interface Tester {
        opencv_core.Mat test(opencv_core.Mat source);
    }

    final static Tester LOW_LEVEL_CRASHING = source -> {
        final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
        opencv_imgproc.GaussianBlur(uMat, uMat, KSIZE, 0.0);
        final opencv_core.Mat result = uMat.getMat(opencv_core.ACCESS_RW);
//        result.close();
//        uMat.close();
        return result;
    };

    final static Tester SIMPLE_CLOSING = source -> {
        final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
        opencv_imgproc.GaussianBlur(uMat, uMat, KSIZE, 0.0);
        return toMatAndClose(uMat);
    };

    final static Tester LOW_LEVEL_IN_PLACE_ON_CLONE = source -> {
        final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
        printMat("source", source);
        final opencv_core.UMat uMatClone = uMat.clone();
        printMat("uMat", uMat);
        opencv_imgproc.GaussianBlur(uMatClone, uMatClone, KSIZE, 0.0);
        printMat("uMatClone", uMatClone);
        opencv_core.Mat mat = uMatClone.getMat(opencv_core.ACCESS_RW);
        printMat("source", source);
        printMat("mat", mat);
        opencv_core.Mat result = mat.clone();
        printMat("result", result);
        mat.close();
        uMatClone.close();
        printMat("uMat", uMat);
        printMat("uMatClone", uMatClone);
        return result;
    };

    final static Tester LOW_LEVEL_IN_OTHER = source -> {
        final opencv_core.UMat uMat = source.getUMat(opencv_core.ACCESS_RW);
        printMat("source", source);
        printMat("uMat", uMat);
        opencv_core.UMat uResult = new opencv_core.UMat();
        opencv_imgproc.GaussianBlur(uMat, uResult, KSIZE, 0.0);
        printMat("uMat", uResult);
        opencv_core.Mat mat = uResult.getMat(opencv_core.ACCESS_RW);
        printMat("source", source);
        printMat("mat", mat);
        opencv_core.Mat result = mat.clone();
        printMat("result", result);
        mat.release();
        uResult.release();
        printMat("uMat", uMat);
        return result;
    };

    private static final Tester TESTER = LOW_LEVEL_CRASHING;

    private static void printMat(String name, opencv_core.Mat mat) {
        System.out.printf("%-9s %s, address 0x%x%n",
                name + ":", mat, mat.address());
    }

    private static void printMat(String name, opencv_core.UMat mat) {
        System.out.printf("%-9s %dx%d %s, address 0x%x, buffer %s%n",
                name + ":",
                mat.isNull() ? -1 : mat.rows(),
                mat.isNull() ? -1 : mat.cols(),
                mat,
                mat.address(), mat.asByteBuffer());
    }

    public static void main(String[] args) throws FileNotFoundException, InterruptedException {
//        opencv_core.setUseOpenCL(false);
        System.out.println("OpenCL existance: " + opencv_core.haveOpenCL());
        System.out.println("OpenCL usage: " + opencv_core.useOpenCL());
        if (args.length < 2) {
            System.out.printf("Usage: %s source_image target_image%n", SimpleUMatUsageTest.class);
            return;
        }
        final String sourceFile = args[0];
        final String targetFile = args[1];
        if (!Files.exists(Paths.get(sourceFile))) {
            throw new FileNotFoundException(sourceFile);
        }
        for (int testCount = 0; testCount < 150; testCount++) {
            final opencv_core.Mat source = opencv_imgcodecs.imread(sourceFile);
            System.out.println("Test #" + testCount);

            final opencv_core.Mat result = TESTER.test(source);
            opencv_imgcodecs.imwrite(targetFile, result);
            System.err.println("Start gc...");
            for (int k = 0; k < 10; k++) {
                System.gc();
                Thread.sleep(10);
            }
            System.err.println("End gc");
        }
    }
}

On RGB test picture 1518x1500, on my computer, it leads to error already in 1st gc():

OpenCL existance: true OpenCL usage: true Test #0 Start gc... OpenCV: terminate handler is called! The last OpenCV error is: OpenCV(4.0.0) Error: Assertion failed (u->refcount == 0 && "UMat deallocation error: some derived Mat is still alive") in cv::ocl::OpenCLAllocator::deallocate, file C:\projects\javacpp-presets\opencv\cppbuild\windows-x86_64-gpu\opencv-4.0.0\modules\core\src\ocl.cpp, line 4766

Process finished with exit code -1073740791 (0xC0000409)

[Daniel-Alievsky]

In addition, my workaround has a performance problem. How can I convert UMat to Mat with preserving previous UMat content? Now I see the only safe way toMatAndClose(u.clone()) but it means double copying: cloning UMat ("u"), converting clone by getUMat (some "m"), cloning "m" to result, releasing both "u" clone and "m", Is there better way?

[saudet]

That sounds like a limitation of OpenCV. We just need to keep a reference to both objects and that should work as expected. BTW, we could use PointerScope for that, for example, this probably works:

try (PointerScope scope = new PointerScope()) {
    UMat uMat = source.getUMat(ACCESS_RW);
    GaussianBlur(uMat, uMat, KSIZE, 0.0);
    Mat clone = uMat.getMat().clone();
    scope.detach(clone);
    return clone;
}

We could also keep the scope alive, but there might not be any benefit over just carrying the UMat around:

try (PointerScope scope = new PointerScope(false)) {
    UMat uMat = source.getUMat(ACCESS_RW);
    GaussianBlur(uMat, uMat, KSIZE, 0.0);
    Mat mat = uMat.getMat();
    // return both scope and mat somehow...
}

Sounds good? BTW, in the case of GaussianBlur() we could simply reuse the source Mat on output, avoiding any additional allocation.

[Daniel-Alievsky]

I thought that it is possible just to check this situation in deallocator, when it is called from finalizer. Really, there is no problem here - garbage collector will remove both UMat and derived Mat. The only problem is that your finalizer does not check this situation and throws an exception, that enforce stopping all JVM.

In any case, tell me, please, answer the simple question. I have UMat "uMat", result of some GPU algorithm. The only thing that I need - to copy this UMat into usual Mat (variable "mat"), stored in usual memory (and available in Java as ByteBuffer), that have no any connection with the previous UMat, and "forget" about this UMat - it was temporary and I don't need it now. The result of my function must be Mat. What is the CORRECT way to do this?

mat = uMat.getMat(opencv_core.ACCESS_RW);

It is recommended, but INCORRECT. When I (or gc) will close uMat, it will lead to error, because mat is still alive.

mat = toMatAndClose(uMat.clone())

(see my function toMatAndClose) is correct, but means 2 extra copying and required double space in GPU and CPU memory.

mat = toMatAndClose(uMat)

is little better, but unsafe, if I amn't sure that uMat is a newly created GPU matrix: for example, it is, maybe, derived from other Mat, that I must not release and this moment.

[saudet]

We need to do the same as done in C++. JavaCPP doesn't do anything special! If there is a C++ example that does what you need, do the same, and it will work.

[Daniel-Alievsky]

You should understand the difference between C++ and Java languages.

In C++, getMat is a good, stable solution. In this language we have only 2 alternatives. 1st: we create Mat or UMat, use it, and then destroy it: destructor is called. Moreover, C++ helps to call destructor in time. 2nd: we forgot to destroy the object. In this case, we will see memory leak, in this case - very quick memory leak (images usually spend megabytes memory).

In Java, we usually create objects and pass them to other classes, without worrying about destroying: it is the function of garbage collector. If I have a String and need to convert to upper case, I just call s.toUpperCase() and don't think, who and when will free memory of the source string "s". What can I do with UMat and Mat?

Yes, I can call uMat.getMat and return it. It seems to be normal Java practice: I performed some image processing in GPU and now need to pass the results, usual matrix in memory, to other classes for storing, showing, maybe further processing. While usual work, it is a normal solution: you implemented good processing native references in Pointer class, and garbage collector successfully utilize them. And all works fine, while we use Mat class and other javacpp classes (Scalar, CvArr, etc). But with UMat, the simple call "uMat,getMat" will be a SERIOUS BUG! As I've shown, if I'll just return uMat.getMat, it will work in 99% cases, but when Java will start garbage collection, JVM will crash at all with low-level error message! See LOW_LEVEL_CRASHING in my test.

Of course, if my goal is to write a simple utility with 100 lines of code, I can remember about this issue and try to call "close" for all my Mat and UMat instances in correct order. But I work over complex software with megabytes of source code, and I need to create library of ready-to-use functions, performing some image processing and returning results in a form of Mat / ByteBuffer.

So, let me repeat my simple question. I have some UMat: result of some image processing algorithm.

UMat uMat = myComplexAlgorithm(some matrices, vectors, ...)

I need to convert it to Mat and return Mat:

public opencv_core.Mat myFunction(some data) {
    final opencv_core.UMat uMat = myComplexAlgorithm(some data);
    final opencv_core.Mat result = ??? // not uMat.getMat(opencv_core.ACCESS_RW) !!!
    return result;
}

How can I do this? Is here better way, than my workaround method: result = toMatAndClose(uMat.clone()) ?

Now I found another solution:

public opencv_core.Mat myFunction(some data) {
    final opencv_core.UMat uMat = myComplexAlgorithm(some data);
    final opencv_core.Mat clone = new opencv_core.Mat(uMat.rows(), uMat.cols(), uMat.type());
    uMat.copyTo(clone);
    return clone;
}

Consult me please: is it a good stable solution for Java, without risk of crash in garbage collector?

In any case, I think is makes sense to do something in your javacpp bridge: as a minimum, to warn in comments about the gc() problem described above, as a maximum, to solve this problem by more accurate processing garbage reference and catching possible problem with UMat deallocation error

[saudet]

It is never a good idea to rely on the garbage collector for managing resources. We should do the same in Java as in C++. The garbage collector is there only when the user fails to release resources manually. For resources managed by JavaCPP, we can call the destructor with deallocate(), or let it leak resources with deallocate(false). It will work exactly the same as in C++, nothing more, nothing less. If you need something better than what OpenCV offers in C++, then we'll need to think about a solution, but that solution cannot involve the garbage collector. AFAIK, PointerScope is the best we have, and in my opinion should be at least a good starting point, so please give it a try! However, given that this is an issue with OpenCV's API, that is, it will behave exactly the same in C++, this is something that should be fixed upstream.

[Daniel-Alievsky]

I think you will agree that garbage collector should not lead to crash of JVM. I agree that deallocating native memory in-time is a good practice even in Java, and, of course, our software tries to do this - but sometimes it is very difficult to guarantee this while Java-style programming. Yes, it could lead to little decreasing performance: if your finalizing code will not work in-time, OS can occupy a lot of virtual memory for storing extra native data. The worst result - we will see "out of memory" error, though we would have enough memory, if the system would perform garbage collection in time. But the result that we must never see - total crash of JVM as a result of ordinary garbage collection, as in my test. Are you agree?

Early releases of JDK 1.6 had similar problem with deallocation of mapped memory, allocated by FileChannel.map method. Sometimes, garbage collection of such block led to exception, and JVM crached. I created bug report to Sun and, of course, they fixed it.

Also, if it is possible, can you reply to my last question? It seems that this solution is absolutely stable and doesn't involve extra copying, unlike workaround with getMat:

public static opencv_core.Mat toMat(opencv_core.UMat u) {
    final opencv_core.Mat clone = new opencv_core.Mat(u.rows(), u.cols(), u.type());
    u.copyTo(clone);
    return clone;
}

Is it right? I believe you know OpenCV architecture much more deeply than me. Is here "clone" an absolutely new copy of uMat, without any internal references that can lead to problems? Can I safely release uMat after copying to new Mat as in this toMat method?

[saudet]

If you need to guarantee that the garbage collector won't cause a crash in your program, you could disable it by setting the "org.bytedeco.javacpp.nopointergc" system property to "true": http://bytedeco.org/javacpp/apidocs/org/bytedeco/javacpp/Pointer.html#referenceQueue If that is a good enough compromise for you, feel free to use it like that. If you know of a way to fix this in general or come up with a hack for just this case, please send a pull request and I'll merge your fix.

Creating a clone or a copy is exactly the same thing, and that's a good workaround, sure, but there is overhead. The new copy/clone is independent yes, it won't have any references to the UMat.

[Daniel-Alievsky]

You didn't understand me well - I don't need that gc will work correctly for me. I already know about this and will avoid getMat function. Really, it is a very, very dangerous method in Java. If just seems to be a serious bug that should be corrected in JavaCPP UMat support.

I don't sure, but I believe this bug can be fixed relatively simply - you just need to check dangerous situation in your finalizer of UMat and don't try to release it. (Or just catch this exception.) It will be released in any case while releasing parent Mat. In any case, when you finalize your objects for GC, if you suppose that some exceptions are possible, you must catch them: in other case, JVM will crash, and it is the worst possible result. It seems you do not do this: I don't see any catching exceptions in your Pointer.DeallocatorThread class; but, maybe, I don't understand your logic enough.

[saudet]

If the Mat object returned by UMat.getMat() should not be released, then that's a bug in OpenCV. If we create a Mat using, for example, an external data pointer, releasing it does nothing. If you believe that's what it should also do for objects returned by UMat.getMat() as well, then we should open an issue about that upstream.

[saudet]

If you don't want the GC to release the UMat, just call deallocate(false) on it, that should be enough, no? Users are free to do what they want, including crashing their applications. That's a usability issue with OpenCV! Leaking memory on purpose is not a solution. If you do think of a solution that doesn't involve leaking memory, please let me know.

[saudet]

I think I finally understand what you're saying. A C++ exception is thrown from a destructor and that causes the JVM to terminate, correct? For one thing, C++ exceptions should never be thrown from destructors, so this is most definitely a bug in OpenCV that should get fixed upstream. About what we could do to work around this in JavaCPP, well I don't know, but forcing an overhead on all users to try to catch exceptions that should never get thrown in the first place because they cause undefined behavior anyway doesn't seem like a good idea to me...

[Daniel-Alievsky]

Did you run my test? It is very, very simple. And it leads to termination of JVM, when I use getMat, with almost 100% probability (if I call gc() manually). I believe it is not a bug of OpenCV C++ code, it seems to be a bug of JavaCPP that "thinks" that exceptions are impossible while UMat.release. They are possible! "UMat deallocation error: some derived Mat is still alive"

Your users cannot catch such exceptions, but you can: in your finalization code. And it is better not to catch an exception, but check the situation, that UMat has a derived Mat, and not to try to release it. You are right that JavaCPP users should release all native memory in time :) In any case, if the user will forget to release Mat/UMat pair correcrly, garbage collector will release Mat, and it will be enough: a connected UMat will be released automatically. If I understand all correctly, Finalization code in Pointer.DeallocatorThread must help to avoid memory leak, but not "help" to destroy all application.