Added finalize back in

[sgjava]

Finalize added back in and nativeObj safety check in case delete() or release() already called.

[apavlenko]

Instead of merging this one I suggest reverting #4014.
This one still mixes release with delete that are totally different things!

[vpisarev]

agree. reverting #4014 is also more correct in terms of git practices, we can be sure then that we get back exactly to the previous state.

[apavlenko]

Replacing with #4031 and closing.

[sgjava]

This removed finalize and left n_delete in release(). Instead of doing random reverts, how about I fix them properly and submit a pull request?

[apavlenko]

@sgjava , the solution that was implemented before your changes allows both manual memory clean-up via Mat::release() and less efficient but automated finalize() API.
I see no improvements in removing more traditional Java world finalize() approach in order to force people using manual Mat::release(). Also I consider as a conceptual error mixing Mat::release() with object deletion.

[apavlenko]

If you really know a better solution than the existing one - let's discuss it in details before merging changes into almost released code.

[sgjava]

finalize() is not traditional and is in fact bad practice as I've pointed out several times. It can behave differently on different JVM implementations and versions. In fact, it is not called at all a lot of times and causes the Finalizer queue to get backed up besides performance issues. If finalize were best practice it would be used for JDBC connections, file I/O. etc. to clean up resources. Even Oracle says not to use them http://docs.oracle.com/cd/E13188_01/jrockit/docs142/devapp/codeprac.html#998529

That being said I'd still like to make the changes I've made, but keep finalize since you guys want it there. Here's what I propose:

o Keep finalize()
o Mat.release and Mat.delete will be separate, but I'd suggest a a new helper method like releaseAndDelete() the wraps both, since 99% of the time that's what you are doing.
o Use public modifier on nativeObj so it can be modified by delete() to set it to 0. This way if delete() is called more than once (you call delete() explicitly, but finalize() tries to do it again) you get a Java exception instead of a JVM crash.

Let me know if this sounds good and I can make the changes once all the reverts are done.

[apavlenko]

Since created Java Mat object should keep the corresponding native Mat object and they should be destroyed simultaneously. So users should never call delete() and put Java object to invalid state. Both nativeObj pointer and delete() method were made protected intentionally to prevent calls from user code!
When user wants to free up the memory occupied by the corresponding native object data - it's enough to call release() (this way work in C++ API as well); After release() Mat object is still valid, can be passed to further OpenCV calls (new memory will be allocated), but occupies only few bytes in native memory.

[sgjava]

Yea, but the way of disposing both Java Object and native memory at the same time are flawed since it's based on finalize(). Native memory is the major concern. If you use finalize() then you are guaranteed that Object will be there possibly until JVM exit which could be days or weeks depending on the application. You may say this is only a few bytes, but why leave unused Objects in memory when they should have been GC?

Any ways, I agreed with you to let release() only do Mat::release. It would still be nice to have public access to delete() because I'll just patch the source on my end to remove finalize() since I don't want to use a broken implementation. You have to remember there's no protection against calling release() twice. You will get *** Error in `/usr/lib/jvm/jdk1.8.0/bin/java': malloc(): memory corruption: 0x00007fd814230fb0 *** and crash the JVM. This should be protected by a check of nativeObj.

[apavlenko]

Again: the delete function (destroying the native Mat) shouldn't be called until the Java Mat is being destroyed, both Java and native Mat objects should have the same live time. That's why it will not be public.
Mat::release() without deleting native object can be called as many times as needed - it just makes Mat object empty but still valid.
Applications with sophisticated memory use can call Java garbage collector manually to not wait for weeks for it automatic run...

[sgjava]

Yea, I understand how release and delete works. I'm working on a page describing the memory issues using NetBeans profiler and Valgrind. There are very few corner cases that would require explicit GC (I've never used it in 20 years of Java development). "The reason everyone always says to avoid System.gc() is that it is a pretty good indicator of fundamentally broken code. Any code that depends on it for correctness is certainly broken; any that rely on it for performance are most likely broken." I believe that is the case here.

http://stackoverflow.com/questions/2414105/why-is-it-bad-practice-to-call-system-gc