[BUG] stop shipping aidl implementation headers in the NDK

[atsushieno]

Description

There was an API breaking change that was suddenly introduced at NDK 21.3..6528147, and to make it worse, this breaks ANY native binder service implementation derived from BnCInterface which is automatically generated by aidl(.exe).

$ diff -ur ~/Android/Sdk/ndk/21.2.6472646/sysroot/usr/include/ ~/Android/Sdk/ndk/21.3.6528147/sysroot/usr/include/ | grep SharedRefBase
     std::weak_ptr<SharedRefBase> mThis;
+    // Use 'SharedRefBase::make<T>(...)' to make. SharedRefBase has implicit

The problematic file is binder_interface_utils.h.

With NDK 21.3..6528147, it became simply impossible to define a service implementation class that is derived from the abstract class for the AIDL generated by aidl(.exe), because the generated class itself is abstract and therefore SharedRefBase::make() does not accept it. And we cannot define the derived class without making appropriate base constructor to BnCInterface which is now simply impossible.

To my understanding, this new operator must not be defined at SharedRefBase.

As long as I searched for any usage of BNCInterface at cs.android.com, it is totally untested. https://cs.android.com/search?q=BNCInterface

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

• NDK Version: 21.3.6528147
• Build system: sysroot/usr/include
• Host OS: Linux (Ubuntu 20.04)
• ABI: i686 (should be any)
• NDK API level: 29
• Device API level: 29 (should be irrelevant)

[DanAlbert]

The API owner will be responding with a useful answer in a bit, but it sounds like the immediate action to be taken here is for me to revert https://android-review.googlesource.com/c/platform/frameworks/native/+/1235494 in the r21 branch to restore API compatibility within the major release. This got included along with the rest of the R APIs.

[smore-lore]

Thank you for reporting this issue.

In your search, you can see the AIDL compiler code, and in the various tests of this compiler, we ensure compatibility between the binder NDK. The main test in platform for this code is CtsNdkBinderTestCases, but it is also used extensively.

The problem here is a mismatch between the AIDL compiler and the binder NDK. Specifically, these two changes were made in Android 11:
https://android-review.googlesource.com/c/platform/frameworks/native/+/1235494
https://android-review.googlesource.com/c/platform/system/tools/aidl/+/1234468

As you can see, you have the latest NDK headers, but not the latest version of AIDL, but this shouldn't be a problem you need to deal with. The plan is to revert the NDK API change, and file a separate platform issue to ensure this type of mismatch doesn't happen again. Thanks!

edit: forward looking platform issue filed at http://issuetracker.google.com/160624671

[DanAlbert]

A question that came up while discussing this, how are you actually consuming this? @smore-lore says that AGP doesn't have support for NDK aidl. Do you have your own build system that does support it?

[atsushieno]

Not really; I rarely update the API, so I check in the generated code along with the aidl file, and whenever I need it I simply run a shell script that only runs aidl tool.

[DanAlbert]

I see. Well, speaking with @smore-lore today I believe the short term fix for you is to use an up to date version of aidl. The problem is that there's unwanted coupling between the headers in the NDK and the aidl compiler. The issue you're running into is that you're using an older version of aidl with the new headers. If you use an aidl from build-tools 30 it should work. Let us know if it doesn't.

[atsushieno]

I should have included the build-tools version... it was with 30.0.0. I'm seeing #ifdef BINDER_STABILITY_SUPPORT which I believe is new in the latest version. I'm not sure if loosely-coupled build-tools/aidl version vs. NDK API is very problematic...maybe it would be acceptable to most people if appropriate coding workaround is available, I guess.

In the meantime I can stay with older NDK setup until the fix lands in newer versions (it's actually kind of complicated; I also use GitHub Actions which always removes older NDKs from the image, which I don't think is a good idea...). Thanks for the hint on possible workaround anyways.

[atsushieno]

I believe the forementioned issue is fixed in build-tools r30.0.2. Maybe this can be closed as well?

Edit: I revisited the actual issue comment 654437828 and noticed that having up-to-date build-tools is not an expected fix for you teams. But as the comment on the issue above says, the problem itself is fixed appropriately anyways.

[DanAlbert]

We're still waiting to get the fix that defends against the problem recurring into r21. Will close after that.