Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] When using inout parameter in AIDL, the code generator is not working properly #5079

Closed
MrAlbin opened this issue Sep 3, 2020 · 0 comments · Fixed by #5726
Closed
Assignees
Labels
Area: Bindings Issues in Java Library Binding projects. bug Component does not function as intended.

Comments

@MrAlbin
Copy link

MrAlbin commented Sep 3, 2020

Steps to Reproduce

  1. Add a function with an inout parameter to an AIDL file:
    for example:
    int GetStringArray(inout String[] stringArray);
  2. Implement the function in your Service.
  3. Compile.

Expected Behavior

The code generator should produce:

...
protected override bool OnTransact (int code, global::Android.OS.Parcel data, global::Android.OS.Parcel reply, int flags)
{
	switch (code) {
	...
	case TransactionGetStringArray: {
		data.EnforceInterface (descriptor);
		String [] arg0 = default (String []);
		arg0 = data.CreateStringArray ();
		var result = this.GetStringArray (arg0);
		reply.WriteNoException ();
		reply.WriteInt (result);
		reply.WriteStringArray (arg0);
		}
	...
	}
...
}

Actual Behavior

The code generator produces:

...
protected override bool OnTransact (int code, global::Android.OS.Parcel data, global::Android.OS.Parcel reply, int flags)
{
	switch (code) {
	...
	case TransactionGetStringArray: {
		data.EnforceInterface (descriptor);
		String [] arg0 = default (String []);
		arg0 = data.CreateStringArray ();
		var result = this.GetStringArray (arg0);
		reply.WriteNoException ();
		reply.WriteInt (result);
		data.WriteStringArray (arg0);
		}
	...
	}
...
}

What goes wrong:
It should not be data.WriteStringArray (arg0); but reply.WriteStringArray (arg0);.

Version Information

Microsoft Visual Studio Professional 2019
Version 16.7.2
VisualStudio.16.Release/16.7.2+30413.136
Microsoft .NET Framework
Version 4.8.03752

Installierte Version: Professional

Visual C++ 2019 00435-60000-00000-AA836
Microsoft Visual C++ 2019

Xamarin 16.7.000.440 (d16-7@358f3c6)
Visual Studio-Erweiterung, um Entwicklung für Xamarin.iOS und Xamarin.Android zu ermöglichen.

Xamarin Designer 16.7.0.495 (remotes/origin/d16-7@79c0c522c)
Visual Studio-Erweiterung zum Aktivieren der Xamarin Designer-Tools in Visual Studio.

Xamarin Templates 16.7.85 (1bcbbdf)
Templates for building iOS, Android, and Windows apps with Xamarin and Xamarin.Forms.

Xamarin.Android SDK 11.0.2.0 (d16-7/025fde9)
Xamarin.Android Reference Assemblies and MSBuild support.
Mono: 83105ba
Java.Interop: xamarin/java.interop@1f3388a
ProGuard: Guardsquare/proguard@ebe9000
SQLite: xamarin/sqlite@1a3276b
Xamarin.Android Tools: xamarin/xamarin-android-tools@017078f

Xamarin.iOS and Xamarin.Mac SDK 13.20.2.2 (817b6f72a)
Xamarin.iOS and Xamarin.Mac Reference Assemblies and MSBuild support.

@grendello grendello added the Area: Bindings Issues in Java Library Binding projects. label Sep 4, 2020
@grendello grendello added this to the Under Consideration milestone Sep 4, 2020
jonpryor pushed a commit that referenced this issue Mar 26, 2021
Fixes: #4717
Fixes: #5079

Context: https://cs.android.com/android/platform/superproject/+/af1f0005723416a36e025fded7eccb2fd8c777de:system/tools/aidl/generate_java_binder.cpp;l=358
Context: https://cs.android.com/android/platform/superproject/+/af1f0005723416a36e025fded7eccb2fd8c777de:system/tools/aidl/generate_java_binder.cpp;l=486
Context: https://cs.android.com/android/platform/superproject/+/af1f0005723416a36e025fded7eccb2fd8c777de:system/tools/aidl/generate_java_binder.cpp;l=646

`Xamarin.Android.Tools.Aidl.CSharpCodeGenerator` did not properly
support `inout` or `out` parameter marshaling of array types.

For `inout` arrays:

	// AIDL
	interface Test {
	    void inoutArray (inout byte [] args);
	}

the emitted `OnTransact()` method needs to write the "output" value
to the `reply` parameter, *not* the `data` parameter:

	// generated C#
	public interface ITest : Android.OS.IInterface {
	    void InoutArray (byte[] args)
	}
	abstract partial class ITestStub : Binder, ITest {
	    protected override bool OnTransact (int code, Parcel data, Parcel reply, int flags)
	    {
	        switch (code) {
	        case TransactionInoutArray: {
	            data.EnforceInterface (descriptor);
	            byte [] arg0 = default (byte []);
	            arg0 = data.CreateStringArray ();
	            this.InoutArray (arg0);
	            reply.WriteNoException ();
	            data.WriteByteArray (arg0);         // SHOULD USE `reply`, not `data`!
	            return true;
	            }
	        }
	    }
	}

Similar-yet-more-so, with `out` array parameters:

	// AIDL
	interface Test {
	    void outArray (out byte [] args);
	}

the `OnTransact()` method *also* needs to support `args` being `null`:

	// generated C#
	public interface ITest : global::Android.OS.IInterface {
	    void OutArray (byte[] args)
	}
	abstract partial class ITestStub : Binder, ITest {
	    protected override bool OnTransact (int code, Parcel data, Parcel reply, int flags)
	    {
	        switch (code) {
	        case TransactionOutArray: {
	            data.EnforceInterface (descriptor);
	            byte [] arg0 = default (byte []);
	            int arg0_length = data.ReadInt();   // NEEDED
	            if (arg0_length < 0) {              // NEEDED
	                arg0 = null;                    // NEEDED
	            }                                   // NEEDED
	            else {                              // NEEDED
	                arg0 = new byte [arg0_length];	// NEEDED
	            }                                   // NEEDED
	            this.OutArray (arg0);
	            reply.WriteNoException ();
	            reply.WriteByteArray (arg0);        // PREVIOUSLY used data, not reply!
	            return true;
	            }
	        }
	    }
	}

*Plus* the `Proxy` implementation needs to support `null`:

	partial class ITestStub {
	    public class Proxy : Java.Lang.Object, ITest {
	        public void OutArray (byte[] args)
	        {
	            var __data  = Parcel.Obtain ();
	            var __reply = Parcel.Obtain ();
	            try {
	                __data.WriteInterfaceToken (descriptor);
	                if (args == null) {                 // NEEDED
	                    __data.WriteInt (-1);           // NEEDED
	                } else {                            // NEEDED
	                    __data.WriteInt(args.Length);   // NEEDED
	                }                                   // NEEDED
	                remote.Transact (ITestStub.TransactionOutArray, __data, __reply, 0);
	                …
	        }
	    }
	}

This is comparable to the Java code the Android Studio produces.
@jonpryor jonpryor added the bug Component does not function as intended. label Apr 13, 2021
@xamarin xamarin locked as resolved and limited conversation to collaborators Jun 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: Bindings Issues in Java Library Binding projects. bug Component does not function as intended.
Projects
None yet
4 participants