Bug 22218 - [generator] Parameter marshaling is not exception-safe.
Summary: [generator] Parameter marshaling is not exception-safe.
Status: VERIFIED FIXED
Alias: None
Product: Android
Classification: Xamarin
Component: General ()
Version: 4.16.0
Hardware: PC Windows
: High normal
Target Milestone: 5.1
Assignee: dean.ellis
URL:
Depends on:
Blocks:
 
Reported: 2014-08-20 01:03 UTC by Jared Kells
Modified: 2015-03-12 17:51 UTC (History)
8 users (show)

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


Attachments
Testcase and devenv.log (12.80 KB, application/x-zip-compressed)
2014-08-20 05:34 UTC, Jared Kells
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:
VERIFIED FIXED

Description Jared Kells 2014-08-20 01:03:18 UTC
When a java exception is thrown the bindings leak jni local references. If running in a native thread, such as a .NET thread pool thread the table is never cleared and the application will eventually crash.

Here is the error message from ADB:

08-20 14:54:49.810: E/dalvikvm(28917): JNI ERROR (app bug): local reference table overflow (max=512)
08-20 14:54:49.810: W/dalvikvm(28917): JNI local reference table (0x79b5d398) dump:
08-20 14:54:49.810: W/dalvikvm(28917):   Last 10 entries (of 512):
08-20 14:54:49.810: W/dalvikvm(28917):       511: 0x4161b528 java.lang.String "GBP"
08-20 14:54:49.810: W/dalvikvm(28917):       510: 0x41619090 java.lang.String "∞"
08-20 14:54:49.810: W/dalvikvm(28917):       509: 0x41619058 java.lang.String "E"
08-20 14:54:49.810: W/dalvikvm(28917):       508: 0x41619180 java.lang.String "£"
08-20 14:54:49.810: W/dalvikvm(28917):       507: 0x41618fd0 java.lang.String "#,##0.###"
08-20 14:54:49.810: W/dalvikvm(28917):       506: 0x415506b8 java.lang.Class<libcore.icu.NativeDecimalFormat>
08-20 14:54:49.810: W/dalvikvm(28917):       505: 0x42630518 java.lang.String "leaky leaky"
08-20 14:54:49.810: W/dalvikvm(28917):       504: 0x42630060 java.lang.String "leaky leaky"
08-20 14:54:49.810: W/dalvikvm(28917):       503: 0x4262fba8 java.lang.String "leaky leaky"
08-20 14:54:49.810: W/dalvikvm(28917):       502: 0x4262f6f0 java.lang.String "leaky leaky"
08-20 14:54:49.810: W/dalvikvm(28917):   Summary:
08-20 14:54:49.810: W/dalvikvm(28917):         4 of java.lang.Class (4 unique instances)
08-20 14:54:49.810: W/dalvikvm(28917):       505 of java.lang.String (505 unique instances)
08-20 14:54:49.810: W/dalvikvm(28917):         1 of java.lang.String[] (2 elements)
08-20 14:54:49.810: W/dalvikvm(28917):         1 of jni_bug.MainActivity
08-20 14:54:49.810: W/dalvikvm(28917):         1 of mono.java.lang.RunnableImplementor
08-20 14:54:49.810: E/dalvikvm(28917): Failed adding to JNI local ref table (has 512 entries)


Here is a a trivial reproduction:

            var thread = new Thread(() =>
                                    {
                                        for (var i = 0; i < 600; i++)
                                        {
                                            try
                                            {
                                                // Throws an exception, invalid format
                                                var format = new SimpleDateFormat("leaky leaky");
                                            }
                                            catch (Exception e)
                                            {
                                            }
                                        }
                                    });
            thread.Run();
Comment 1 Prashant manu 2014-08-20 04:10:47 UTC
Could you please provide us the test steps you followed. If possible please
attach a small complete project that demonstrates this behavior.

Also can you please add the logs from the following places?:
On XS IDE Log  Location: XS>Help> Open Log Directory> IDE.log file (with latest
timestamp)
AndroidTools log: XS->Help->Open Log Directory->AndroidTools.log

If using VS, 
IDE log: Location: C:\User\AppData\Local\Xamarin\Log\ 
(Devenev file with latest timestamp)
Android Trace log: Location: \AppData\Local\Xamarin\Log\XamarinAndroid
Comment 2 Jared Kells 2014-08-20 05:34:59 UTC
Created attachment 7733 [details]
Testcase and devenv.log
Comment 3 Jared Kells 2014-08-20 05:36:40 UTC
Hi, 

I attached a testcase and my devenv.log. An IDE log didn't exist.
Comment 4 Jonathan Pryor 2014-08-20 22:29:00 UTC
The problem is in our binding generator: it doesn't emit exception-safe parameter marshaling code.

Consider the output for the SimpleDateFormat(string) constructor:

> [Register (".ctor", "(Ljava/lang/String;)V", "")]
> public SimpleDateFormat (string pattern) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
> {
> 	if (Handle != IntPtr.Zero)
> 		return;
> 
> 	IntPtr native_pattern = JNIEnv.NewString (pattern);;
> 	if (GetType () != typeof (SimpleDateFormat)) {
> 		SetHandle (
> 				global::Android.Runtime.JNIEnv.StartCreateInstance (GetType (), "(Ljava/lang/String;)V", new JValue (native_pattern)),
> 				JniHandleOwnership.TransferLocalRef);
> 		global::Android.Runtime.JNIEnv.FinishCreateInstance (Handle, "(Ljava/lang/String;)V", new JValue (native_pattern));
> 		JNIEnv.DeleteLocalRef (native_pattern);
> 		return;
> 	}
> 
> 	if (id_ctor_Ljava_lang_String_ == IntPtr.Zero)
> 		id_ctor_Ljava_lang_String_ = JNIEnv.GetMethodID (class_ref, "<init>", "(Ljava/lang/String;)V");
> 	SetHandle (
> 			global::Android.Runtime.JNIEnv.StartCreateInstance (class_ref, id_ctor_Ljava_lang_String_, new JValue (native_pattern)),
> 			JniHandleOwnership.TransferLocalRef);
> 	JNIEnv.FinishCreateInstance (Handle, class_ref, id_ctor_Ljava_lang_String_, new JValue (native_pattern));
> 	JNIEnv.DeleteLocalRef (native_pattern);
> }

In particular, this part (as an example; it's repeated...):

> SetHandle (
> 		global::Android.Runtime.JNIEnv.StartCreateInstance (class_ref, id_ctor_Ljava_lang_String_, new JValue (native_pattern)),
> 		JniHandleOwnership.TransferLocalRef);
> JNIEnv.FinishCreateInstance (Handle, class_ref, id_ctor_Ljava_lang_String_, new JValue (native_pattern));
> JNIEnv.DeleteLocalRef (native_pattern);

Background: JNIEnv.StartCreateInstance() (< Android v3.0) or JNIEnv.FinishCreateInstance() (>= Android v3.0) invokes the actual Java-side constructor/method, which raises an exception from Java.

The problem is that if JNIEnv.StartCreateInstance() or JNIEnv.FinishCreateInstance() (or JNIEnv.Call*Method(), or...) throws an exception, the cleanup IS NOT INVOKED, meaning JNIEnv.DeleteLocalRef(native_pattern) is SKIPPED, which results in....the reported JNI local reference leak.

Dean: we need to update generator so that it uses try/finally to ensure that the cleanup code is actually executed, generating something equivalent to:

> IntPtr native_pattern = JNIEnv.NewString (pattern);;
> ...
> try {
> 	SetHandle (
> 			global::Android.Runtime.JNIEnv.StartCreateInstance (class_ref, id_ctor_Ljava_lang_String_, new JValue (native_pattern)),
> 			JniHandleOwnership.TransferLocalRef);
> 	JNIEnv.FinishCreateInstance (Handle, class_ref, id_ctor_Ljava_lang_String_, new JValue (native_pattern));
> } finally {
> 	JNIEnv.DeleteLocalRef (native_pattern);
> }

This needs to be done *everywhere* that parameters are marshaled.
Comment 5 dean.ellis 2014-09-12 05:27:57 UTC
Fixed in monodroid/master/1de4e48ef77e8
Comment 6 renan jegouzo 2014-09-18 05:04:53 UTC
it will be available in which release ?
Comment 7 renan jegouzo 2014-09-27 03:00:05 UTC
I have this bug https://bugzilla.xamarin.com/show_bug.cgi?id=22444, seems that it sould be fixed, with the fix of this one, when I can expect a fix in stable channel ?
Comment 8 renan jegouzo 2014-10-09 00:42:54 UTC
please, can you stop taking your customers like trashes and tell us in which version we can expect a fix.
Comment 9 Jonathan Pryor 2014-10-24 16:44:02 UTC
@renan: That requires that we actually know which release it'll make it into, and unfortunately our release process isn't quite as predictable as we might like.

As of this point in time, it will be in Xamarin.Android 5.0 (ETA unknown). It may make it into a 4.x release, depending on QA.
Comment 10 Saurabh 2014-11-26 05:50:03 UTC
I have checked this Issue with attached project mentioned in Comment#2. Application successfully deployed and launched on Emulator as well as on devices.

=== Xamarin Studio ===

Version 5.7 (build 596)
Installation UUID: 2939b8b4-8977-42bd-82d6-100275ccd9cd
Runtime:
	Mono 3.12.0 ((detached/b75fa2b)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 312000046

=== Apple Developer Tools ===

Xcode 6.1 (6604)
Build 6A1052d

=== Xamarin.iOS ===

Version: 8.6.0.5 (Business Edition)
Hash: 880cc21
Branch: 
Build date: 2014-11-25 12:12:17-0500

=== Xamarin.Android ===

Version: 5.0.0.0 (Business Edition)
Android SDK: /Users/360_macmini/Library/Developer/Xamarin/android-sdk-mac_x86
	Supported Android versions:
		2.3   (API level 10)
		4.0.3 (API level 15)
		4.1   (API level 16)
		4.2   (API level 17)
		4.3   (API level 18)
		4.4   (API level 19)
		5.0   (API level 21)
Java SDK: /usr
java version "1.6.0_65"
Java(TM) SE Runtime Environment (build 1.6.0_65-b14-462-11M4609)
Java HotSpot(TM) 64-Bit Server VM (build 20.65-b04-462, mixed mode)

=== Xamarin.Mac ===

Version: 1.11.1.201 (Business Edition)

=== Build Information ===

Release ID: 507000596
Git revision: d996e9ba6874a0d64241e43e5e6b06322ce29c84
Build date: 2014-11-25 17:17:54-05
Xamarin addins: 8ca19707b41536a391f53364ee4ff9272711feb0

=== Operating System ===

Mac OS X 10.9.4
Darwin 360-MACMINIs-Mac-mini-2.local 13.3.0 Darwin Kernel Version 13.3.0
    Tue Jun  3 21:27:35 PDT 2014
    root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64