jamesathey / FastAndroidCamera

Camera preview callbacks with less overhead on Xamarin.Android.
Apache License 2.0
60 stars 16 forks source link

OutOfMemoryError Exception #2

Closed omares closed 9 years ago

omares commented 9 years ago

Hey,

when using FastAndroidCamera and returning to the view multiple times ill receive an out of memory error. As soon as i stop returning bytes to the buffers in the callback the issue is fixed. It seems the handle is not properly released? And clue where this comes from?

I am testing with the example app from my own library https://github.com/rebuy-de/rb-forms-barcode

[mono] Unhandled Exception:
[mono] Java.Lang.OutOfMemoryError: Exception of type 'Java.Lang.OutOfMemoryError' was thrown.
[mono]   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x00000] in <filename unknown>:0 
[mono]   at Android.Runtime.JNIEnv.CallStaticObjectMethod (IntPtr jclass, IntPtr jmethod) [0x00063] in /Users/builder/data/lanes/1353/ac29b2c6/source/monodroid/src/Mono.Android/src/Runtime/JNIEnv.g.cs:1144 
[mono]   at Android.OS.Looper.get_MainLooper () [0x0002d] in /Users/builder/data/lanes/1353/ac29b2c6/source/monodroid/src/Mono.Android/platforms/android-21/src/generated/Android.OS.Looper.cs:35 
[mono]   at Android.Runtime.AndroidEnvironment.GetDefaultSyncContext () [0x00000] in /Users/builder/data/lanes/1353/ac29b2c6/source/monodroid/src/Mono.Android/src/Runtime/AndroidEnvironment.cs:181 
[mono]   at System.AndroidPlatform.GetDefaultSyncContext () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.SynchronizationContext.get_Current () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.Tasks.AwaiterActionContinuation.Execute () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.Tasks.Task.ProcessCompleteDelegates () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.Tasks.Task.Finish () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.Tasks.Task.ThreadStart () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.Tasks.Task.Execute () [0x00000] in <filename unknown>:0 
[mono]   at System.Threading.Tasks.TpScheduler.TaskExecuterCallback (System.Object obj) [0x00000] in <filename unknown>:0 
[mono]   --- End of managed exception stack trace ---
[mono] java.lang.OutOfMemoryError
[mono]  at dalvik.system.NativeStart.run(Native Method)
omares commented 9 years ago

Ok nvm it seems to be something else, i will go deeper to be sure if its FastAndroidRelated. Closing the issue until knowing more.

omares commented 9 years ago

Ok now i am sure its FastAndroidCamera related. As soon as i call camera.AddCallbackBuffer(new FastJavaByteArray(buffersize)); the memory starts increasing after each page visit until exploding.

omares commented 9 years ago

Further insights, even a simple instantiation of the object leads to the error.

This is my minimal startPreview method

public void StartPreview(PreviewFrameCallback previewFrameCallback)
{
    try {
        var camera = scannerCamera.OpenCamera();

        var parameters = camera.GetParameters();
        int numBytes = (parameters.PreviewSize.Width * parameters.PreviewSize.Height * ImageFormat.GetBitsPerPixel(parameters.PreviewFormat)) / 8;

        for (int i = 0; i <= 3; i++) {
            new FastJavaByteArray(numBytes);
        }

    } catch (Exception ex) {
        this.Debug("Unable to start preview.");
        this.Debug(ex.ToString());
    }
}
omares commented 9 years ago

The Dispose Method of FastJavaByteArray is never called for me, even not the destructor. Seems like the objects are kept forever, ill investigate further.

Even when explicitly calling the Dispose Method the memory grows, my guess is there is somewhere a pointer issue but so far my knowledge of the implementation is to shallow to see the error :(

jamesathey commented 9 years ago

In your StartPreview(), I don't see the FastJavaByteArray get added to anything. I usually call

for (int i = 0; i < NUM_BUFFERS; ++i)
    camera.AddCallbackBuffer(new FastJavaByteArray(numBytes));

camera.SetNonMarshalingPreviewCallback(this);

In my own code, I do that after

camera.StartPreview()
omares commented 9 years ago

Hey james, that example code was just for showing that even without using or assigning the FastJavaByteArray the memory usage grows. I am still not 100% sure if i fucked up somehow :)

omares commented 9 years ago

Something just keeps a reference on the FastJavaByteArray objects which stack up when reopening the camera view. They simply do not get released expect when disposing them manually, which is impossible in the preview case as they get added to the camera buffer :(

omares commented 9 years ago

Found a workaround for the issue by manually disposing every instance of FastJavaByteArray:

PreviewFrameCallback has a reference on the last buffer and clears it before creating a new one.

namespace Rb.Forms.Barcode.Droid.View
{
    public class PreviewFrameCallback : Java.Lang.Object, INonMarshalingPreviewCallback, ILog
    {
        private FastJavaByteArray buffer;

        async public void OnPreviewFrame(IntPtr data, AndroidCamera camera)
        {
            if (buffer != null) {
                buffer.Dispose();
                buffer = null;
            }

            buffer = new FastJavaByteArray(data);

            // code here

            camera.AddCallbackBuffer(buffer);
        }

    }
}

Buffer setup is solved by adding the buffers to an IList and disposing the objects when requested.

namespace Rb.Forms.Barcode.Droid.Camera
{
    public class CameraConfigurator : ILog, IDisposable
    {
        private IList<FastJavaByteArray> buffers = new List<FastJavaByteArray>();

        ~CameraConfigurator()
        {
            Dispose(false);
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        public AndroidCamera Configure(AndroidCamera camera)
        {
            var parameters = camera.GetParameters();

            if (buffers.Count == 0) {
                var buffersize = calculateBufferSize(parameters);

                for (int i = 0; i <= 3; i++) {
                    buffers.Add(new FastJavaByteArray(buffersize));
                }
            }

            foreach (var buffer in buffers) {
                camera.AddCallbackBuffer(buffer);
            }
        }

        private void clearBuffers()
        {
            foreach (var buffer in buffers) {
                buffer.Dispose();
            }

            buffers = new List<FastJavaByteArray>();
        }

        protected virtual void Dispose(bool disposing)
        {
            clearBuffers();
        }
    }
}
jamesathey commented 9 years ago

@omares Which commit should I pull from https://github.com/rebuy-de/rb-forms-barcode to get the version of the app with the problem? What are the steps to reproduce?

omares commented 9 years ago

@jamesathey Pull the 0.3.0-beta release https://github.com/rebuy-de/rb-forms-barcode/tree/0.3.0-beta it includes FastAndroidCamera without the applied bugfixes.

Steps to reproduce:

  1. Start the included sample app (android only).
  2. Go to the scanner page.
  3. Tap the "Toggle Preview" multiple times on the bottom left until the memory crash occurs. or
  4. Going to the main page and back to the scanner using the menu multiple times, will also lead to the error.
omares commented 9 years ago

@jamesathey any news on this?

jamesathey commented 9 years ago

@omares I've finally figured all this out.

Every time you tap "Toggle Preview" on your app, 3 new FastJavaByteArrays are constructed, with the large Java arrays to back them. The FastJavaByteArray holds a JNI global reference to the Java byte array. The only way that reference is released is if the object is finalized or explicitly Dispose()'d.

I originally thought that FastJavaByteArray's finalizer must run eventually, but that's not happening for you, and I finally encountered a scenario where it wasn't happening for me, either. The reason that the finalizer is not running, and therefore not calling Dispose(), is because it's the Java VM that's under memory pressure, not the CLR. Without the CLR feeling the pressure, it's never GC'ing, and therefore never finalizing, and never releasing the references to those Java arrays.

I'm going to update the sample code to help prevent others from tripping over this as well. In the meantime, I'll point out that you can avoid some of the contortions you did, because you don't need to worry about Java losing a reference to the arrays just because the CLR no longer cares about them.

        public AndroidCamera Configure(AndroidCamera camera)
        {
            var parameters = camera.GetParameters();

            var buffersize = calculateBufferSize(parameters);

            for (int i = 0; i <= 3; i++)
            {
                using (FastJavaByteArray arr = new FastJavaByteArray(buffersize))
                {
                  // no need to remember the arrays we create here, let the using() block Dispose() them
                  // as soon as they have been added to the Java Camera object. The memory won't
                  // be destroyed as long as Java has a reference to it
                  camera.AddCallbackBuffer(arr);
                }
            }
        }

Similarly, in your PreviewFrameCallback, after you've handed the array back to Java, you can Dispose() immediately (implicitly via using()) and not hold onto any extra references:

namespace Rb.Forms.Barcode.Droid.View
{
    public class PreviewFrameCallback : Java.Lang.Object, INonMarshalingPreviewCallback, ILog
    {
        async public void OnPreviewFrame(IntPtr data, AndroidCamera camera)
        {
            using (FastJavaByteArray buffer = new FastJavaByteArray(data))
            {
                // code here
                camera.AddCallbackBuffer(buffer);
            }
        }
    }
}