Bug 16539 - Bitmap corrupt
Summary: Bitmap corrupt
Status: RESOLVED INVALID
Alias: None
Product: Android
Classification: Xamarin
Component: Mono runtime / AOT Compiler ()
Version: 4.10.1
Hardware: PC Windows
: Normal normal
Target Milestone: ---
Assignee: Jonathan Pryor
URL:
Depends on:
Blocks:
 
Reported: 2013-12-02 01:46 UTC by Andrey Mazurov
Modified: 2013-12-04 15:28 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
corrupt bitmap memory (103.62 KB, image/png)
2013-12-02 20:31 UTC, Andrey Mazurov
Details
Visual Studio 2013 debug window (5.61 KB, text/plain)
2013-12-02 20:36 UTC, Andrey Mazurov
Details
traces.txt (75.36 KB, text/plain)
2013-12-02 20:43 UTC, Andrey Mazurov
Details
android version (251.45 KB, image/png)
2013-12-03 12:39 UTC, Andrey Mazurov
Details
adb logcat (367.60 KB, text/plain)
2013-12-03 13:09 UTC, Andrey Mazurov
Details
logcat (1.21 MB, application/zip)
2013-12-03 20:33 UTC, Andrey Mazurov
Details
Test Application (343.66 KB, application/zip)
2013-12-04 03:07 UTC, Andrey Mazurov
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED INVALID

Description Andrey Mazurov 2013-12-02 01:46:05 UTC
public class TestView : View
{ 

  private Picture picture;

  public override void Draw(Canvas canvas)
  {
    if (picture == null)
    {
      var bitmap = BitmapFactory.DecodeFile("image.png");
      picture = new Picture();
      var canvas1 = picture.BeginRecord(bitmap.Width, bitmap.Height);
      canvas1.DrawBitmap(bitmap, 0, 0, null);
      picture.EndRecord();
    }
    canvas.DrawPicture(picture);
  }
}
------------------------------
sometimes late image picture corrupt and sometimes application crash
if i use next code then i don't have problem with view 

public class TestView : View
{ 

  private Picture picture;
  private Bitmap bitmap;

  public override void Draw(Canvas canvas)
  {
    if (picture == null)
    {
      bitmap = BitmapFactory.DecodeFile("image.png");
      picture = new Picture();
      var canvas1 = picture.BeginRecord(bitmap.Width, bitmap.Height);
      canvas1.DrawBitmap(bitmap, 0, 0, null);
      picture.EndRecord();
    }
    canvas.DrawPicture(picture);
  }
}
------------------------------
Comment 1 Jonathan Pryor 2013-12-02 13:31:33 UTC
When it crashes, what is the stack trace? Can you please provide a complete repro?
Comment 2 Andrey Mazurov 2013-12-02 20:31:57 UTC
Created attachment 5581 [details]
corrupt bitmap memory
Comment 3 Andrey Mazurov 2013-12-02 20:36:15 UTC
Created attachment 5582 [details]
Visual Studio 2013 debug window
Comment 4 Andrey Mazurov 2013-12-02 20:43:01 UTC
Created attachment 5583 [details]
traces.txt
Comment 5 Andrey Mazurov 2013-12-02 20:46:34 UTC
Picture object don't save reference to bitmap and when the bitmap recycled Picture still alive and use for drawing.
Comment 6 Jonathan Pryor 2013-12-02 20:48:51 UTC
Are you running this with Xamarin.Android 4.8 or 4.10?

There are known GC bugs in 4.8 that will cause objects to be prematurely collected, and the SIGSEGV you report should only happen in 4.8, not 4.10.
Comment 7 Andrey Mazurov 2013-12-02 21:49:33 UTC
Xamarin.Android   4.10.01073 (d23a19bf)
Visual Studio plugin to enable development for Xamarin.Android.
Comment 8 Jonathan Pryor 2013-12-03 12:02:24 UTC
Attachment #5583 [details] (traces.txt) looks like an Application Not Responding (ANR) thread dump, not a crash log. Is there a crash log? (Please use `adb logcat`, not the IDE's Application Output window.)

Attachment #5582 [details] (debug window) is odd; I've never seen a SIGSEGV from JNIEnv.CallVoidMethod() on Xamarin.Android 4.10 before. That implies that we're somehow passing an invalid JNI handle to Android, but there would normally be a dalvikvm message about an invalid handle, and there are no relevant dalvikvm messages here.

That said, the SIGSEGV in native code _could_ be this Android bug: https://code.google.com/p/android/issues/detail?id=16644

It's apparently been fixed ("Released" in 2013), but they don't say which Android version got the release. Based on the Released date of June 2013, it was presumably shipped with API-18/Android v4.3.

What's your target Android device? Are you able to reproduce the crash on an Android v4.3+ target?
Comment 9 Andrey Mazurov 2013-12-03 12:39:48 UTC
Created attachment 5589 [details]
android version
Comment 10 Andrey Mazurov 2013-12-03 13:09:17 UTC
Created attachment 5591 [details]
adb logcat
Comment 11 Jonathan Pryor 2013-12-03 15:39:07 UTC
You're target is running Android v4.3, and thus would presumably contain the Android #16644 fix.

I'm thus running out of ideas. My remaining suggestion is to:

1. Enable CheckJNI:

http://android-developers.blogspot.com/2011/07/debugging-android-jni-with-checkjni.html

    adb shell setprop debug.checkjni 1

2. Enable GREF logging:

http://docs.xamarin.com/guides/android/deployment,_testing,_and_metrics/diagnostics/#Global_Reference_Messages

    adb shell setprop debug.mono.log gref

Re-run the app, then please attach the `adb logcat` output.
Comment 12 Andrey Mazurov 2013-12-03 20:33:22 UTC
Created attachment 5596 [details]
logcat
Comment 13 Andrey Mazurov 2013-12-03 20:34:15 UTC
I started application with this environment.
The application did not crashed but all images are corrupted.
Comment 14 Jonathan Pryor 2013-12-03 22:40:22 UTC
Unfortunately, I'm out of ideas. I don't know why your images are being corrupted. :-(

Fortunately you appear to have a workaround.

On a related note, your app is generating a large number of GREFs; there are over 8000 by the end of logcat.txt. GREFs contribute to GC times, so I wouldn't be surprised if your GC pause times were noticeable.

1368 GREFs are due to creating Picture instances. 232 are due to Bitmap instances.

I find the above somewhat confusing, as it doesn't match with the code you originally provided; I would expect the number of created Picture and Bitmap instances to correspond to each other. They do not.
Comment 15 Andrey Mazurov 2013-12-04 03:07:15 UTC
Created attachment 5597 [details]
Test Application

if you click "Refresh TestView" button any times - no problem.
if you click "Kill bitmap" button and click "Refresh TestView"- application crashes.
Comment 16 Jonathan Pryor 2013-12-04 15:28:35 UTC
_Very_ interesting.

So I run the app in GDB, and:

> Program received signal SIGSEGV, Segmentation fault.
> 0x40118dbc in memcpy () from /Users/jon/Downloads/bxc-16539/AndroidApplication3/AndroidApplication3/gdb-symbols/libc.so
> (gdb) bt
> #0  0x40118dbc in memcpy () from /Users/jon/Downloads/bxc-16539/AndroidApplication3/AndroidApplication3/gdb-symbols/libc.so
> #1  0x4038cf7c in ?? () from /Users/jon/Downloads/bxc-16539/AndroidApplication3/AndroidApplication3/gdb-symbols/libskia.so
> #2  0xbeef79f8 in ?? ()
> Cannot access memory at address 0x400
> #3  0xbeef79f8 in ?? ()
> Cannot access memory at address 0x400
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Given that libc.so and libskia.so are at the top of the backtrace, I think this is a symptom of another Skia bug.

Maybe.

Time to dig into the Android source, and...

https://github.com/android/platform_frameworks_base/blob/1abf5d62/graphics/java/android/graphics/Canvas.java#L1138

        native_drawBitmap(mNativeCanvas, bitmap.ni(), left, top,
                paint != null ? paint.mNativePaint : 0, mDensity, mScreenDensity, bitmap.mDensity);
 
`bitmap.ni()` doesn't look good to me... What's native_drawBitmap() do?

https://github.com/android/platform_frameworks_base/blob/1abf5d62/core/jni/android/graphics/Canvas.cpp#L464

native_drawBitmap() in turn uses SkCanvas::drawBitmap():

https://github.com/android/platform_external_skia/blob/7a4d6a99e99d8be9763fe9f00dca80a67f4ebcef/include/core/SkCanvas.h#L674

Unfortunately SkCanvas::drawBitmap() is virtual, but if we guess a bit I _think_ that we'll actually be using SkPictureRecord::drawBitmap:

https://github.com/android/platform_external_skia/blob/7a4d6a99e99d8be9763fe9f00dca80a67f4ebcef/src/core/SkPictureRecord.h#L59
https://github.com/android/platform_external_skia/blob/7a4d6a99e99d8be9763fe9f00dca80a67f4ebcef/src/core/SkPictureRecord.cpp#L891

This is where things get weird, as my local checkout of AOSP differs from git. In any event, the SkBitmap instance is recorded in SkPictureRecord::addBitmap:

https://github.com/android/platform_external_skia/blob/7a4d6a99e99d8be9763fe9f00dca80a67f4ebcef/src/core/SkPictureRecord.cpp#L1241

Now let's revisit Canvas.drawBitmap(), which invokes `bitmap.ni()`. That returns an integer:

https://github.com/android/platform_frameworks_base/blob/1abf5d62/graphics/java/android/graphics/Bitmap.java#L1610

Also note the Bitmap.BitmapFinalizer type, which has a Java finalizer which disposes of the bitmap.

https://github.com/android/platform_frameworks_base/blob/1abf5d62/graphics/java/android/graphics/Bitmap.java#L1538

TL;DR: Based on the above, we can deduce the architecture, and what the bug is.

The bug isn't in Xamarin.Android. It's in Android.

The architecture is as follows: the Java Bitmap type contains an int field which is a pointer to a C++ SkBitmap instance. The SkBitmap instance is tied to the Bitmap instance, via Bitmap.BitmapFinalizer. Meaning when the the Java Bitmap is finalized, the SkBitmap is destroyed.

Which brings us to Canvas and (presumably, hopefully) SkPictureRecord (via Picture). When Canvas.drawBitmap() is used on a Picture instance, the Bitmap is _not_ "deep" copied. Instead, a pointer to the SkBitmap instance is recorded in the SkPictureRecord instance, and this instance is reused in the future.

Which brings us the order of events:

1. Create Picture.
2. Create Bitmap (creates SkBitmap).
3. Call Picture.BeginRecord(), which creates a Canvas & SkPictureRecord.
4. Call Canvas.DrawBitmap(), which links the SkBitmap (2) to the SkPictureRecord (3).
4. Call Canvas.DrawPicture(), which uses the SkPictureRecord (3) and the SkBitmap (2).
5. Call TestView.KillBitmap(). This will release the Bitmap instance and perform a GC.
5.a: The GC will free the Bitmap instance and the corresponding SkBitmap instance.
6. Call Canvas.DrawPicture(), which uses the SkPictureRecord (3) and the (now invalid!) SkBitmap (2).
7. See garbage.

I assert -- but have not tested -- that the same thing would happen in Java. (At least, I see no way for Java to behave differently. Android's Skia use doesn't use GREFs, and keeps Skia & JNI separate, instead storing raw pointers in class fields.)

The fix for this would be for SkPictureRecord to "deep copy" the Bitmap instance so that it doesn't depend upon the instance sticking around. (Or C++ getting a proper GC so that these things can't happen in the first place ;-)

In the meantime, your "workaround" is the only correct thing to do: you MUST keep the Bitmap alive as long as the Picture is kept alive.