Add Android system property support in CoreCLR.

[lateralusX]

Android apps don't have ability to set environment variables from outside app meaning that the only way is to set it directly in code and rebuild app. This cause issues for runtime that heavily depends on environment variables to enabled/disable various runtime features. Rebuilding the app when enabled/disabled runtime features is not feasible.

Android have the concept of system properties. System properties are global properties shared by all apps and offers a way to enabled/disable features in runtime. Android SDK adds some capabilities to set Mono runtime configuration variables through system properties, but currently its defined case by case and by Android SDK. Due to the large amount of configuration variables supported by the runtime, having a dependency on upper SDK's to define what configuration variables to support is not feasible.

This PR adds support to enumerate Android system properties and merge selected properties into our environment variable block, giving access to both Android system properties as well as environment variables.

Android system properties come with a set of limitations that this PR implement mitigations to work around. Before API version 26, property names are limited to 32 characters and the value cannot be longer than 92 characters. Post API version 26, names can be up to 92 characters but values are still limited to 92 characters. dotnet runtime is currently on API level 23, so we need to handle the short property name limitation.

SYSTEM user is by default the only user with ability to set Android system properties, so device needs to be rooted. This is normally not an acceptable requirement, and we need to set the properties on unrooted devices as well. Android handles this by giving the SHELL user permissions to set system properties using the debug. prefix, giving adb shell the ability to set properties on none rooted devices. Processes not running as SYSTEM or SHELL will not be able to set any system properties but will still be able to enumerate all system properties.

Since Android system properties are global they need to be put into different “namespace” and our properties will end up using the dotnet. prefix. To support the debug. prefix, we will enumerate all properties starting with debug.dotnet or dotnet. and add them as environment variables. Due to the limitations in system property name and value length, this PR implements a number of different ways to set environment variables using long names as well as values. If the total property name is under 32 characters, it is possible to append the name directly to the property prefix, matching the environment variable name (without DOTNET_ prefix being case sensitive), like debug.dotnet.DiagnosticPorts. That property name will correspond to DOTNET_DiagnosticPorts environment variable and the value is taken directly from the system property value. It is also possible to put the full environment variable + value as the property value. In this case the property value name is not used and could be named anything unique, like debug.dotnet.diag_ports with the value DiagnosticPorts=123ending up as DOTNET_DiagnosticPorts=123 environment variable. In case the property name and/or value is longer than the allowed max size, it is possible to split up the value in multiple properties using an index, following the property name. Again, the property name will not be used so could be named anything unique. The first property index is .0 and includes the name and the following index (.1 - .n) includes the values concatenated into the final value.

Enumerating the Android system properties during startup will have some overhead, so its currently an opt in feature and needs to be enabled. This is done through the following Android system property, debug.dotnet.enable_sys_props 1.

Example, defining properties using adb shell setprop command:

// Enable use of Android system properties.
adb shell setprop debug.dotnet.enable_sys_props 1

// export DOTNET_DiagnosticPorts=123
adb shell setprop debug.dotnet.DiagnosticPorts 123

// export DOTNET_StressLogFilename=/data/data/myapp/files/stresslog.bin
adb shell setprop debug.dotnet.StressLogFilename /data/data/myapp/files/stresslog.bin

// export DOTNET_SomeLongPropertyNameExceedingMaxLength
adb shell setprop debug.dotnet.long_prop_1 SomeLongPropertyNameExceedingMaxLength=1

// export DOTNET_DiagnosticPorts=123456
adb shell setprop debug.dotnet.diag_ports.0 DiagnosticPorts
adb shell setprop debug.dotnet.diag_ports.1 123
adb shell setprop debug.dotnet.diag_ports.2 456

[jkotas]

As implemented, this won't kick in for environment seen by managed code, and it won't work for NAOT.

Would it be better to "edit" the environment using putenv as the first thing in the process so that all places that the check the environment variables can see the edits?

[lateralusX]

As implemented, this won't kick in for environment seen by managed code, and it won't work for NAOT.

Would it be better to "edit" the environment using putenv as the first thing in the process so that all places that the check the environment variables can see the edits?

That is an alternative as well if we would like wider impact, its mainly moving this code to a place that is thread safe (running on init thread during startup), any good pointers to such a location that will work on both CoreCLR and NAOT?

[jkotas]

any good pointers to such a location that will work on both CoreCLR and NAOT?

It would have to be added as one of the first things to CoreCLR and NAOT startup

[lateralusX]

any good pointers to such a location that will work on both CoreCLR and NAOT?

It would have to be added as one of the first things to CoreCLR and NAOT startup

@jkotas I looked around a little and there are places where we could probably inject calls to write Android system properties into env variables using setenv/putenv in a thread safe way with regards to the runtime execution, but in case we are "embedded" we have no control of other threads in the process that might end up racing with our calls, corrupting the environment in the end, so not sure we can do it in a reliable way covering all scenarios.

For NAOT I assume we could do something in RhConfig, maybe adding an init method or adding a constructor that will run very early since it's a static in src\coreclr\nativeaot\Runtime\startup.cpp.

For CoreCLR I assume we could do it in PAL Initialize method, looks early enough (called from PAL_InitializeCoreCLR) or we could add a static with constructor to src\coreclr\dlls\mscoree\exports.cpp that will also run early before the rest of the code.

Since we would need to share code between NAOT, CoreCLR and potentially Mono accessing Android system properties, then maybe this code should go into native/minipal and since we end up in native/minipal, then maybe we could share code between all runtimes and system.native in the process. That way we don't need to use setenv/putenv, just make sure the code getting env variables are routed through native/minipal implementation to see regular environment variables + any specific things that we would like to represent as env variables, like Android system properties. Feels like this should be rather straightforward and we update just update CoreCLR environ.cpp the call through native/minipal implementation, the same goes for NAOT RhConifg and System.Native pal_environment.c. If we believe this make sense, I can start to move the code in this PR to align to that plan.

[jkotas]

Sounds reasonable to me.

[jkotas]

I am thinking about the production diagnostic use case that motivated this work.

As implemented, it is going to apply to all .NET apps on the Android device. I do not think it is desirable. I expect that the typical use case is going to be focused on diagnosing a specific app. We do not want to affect (or even brick) other apps on the device. Should this property take a name of the app or something, so that it is limited to specific apps only?

We can support a wildcard like * to make this applicable to all apps on the device. However, the use of this wildcard will be discouraged to diagnose issues in production.

[lateralusX]

Good point, let me think about that scenario when I updated the existing PR with low cost abstraction on none Android platforms.

[jkotas]

One more through: We need similar diagnostics capability on Apple platforms. Thoughts about how we can solve it there?

[lateralusX]

On Apple platforms we have mlauch tool that launches that app and can set env variables using its --setenv argument, see https://github.com/dotnet/maui/wiki/Profiling-.NET-MAUI-Apps#ios-mac-etc for some examples on running EventPipe on iOS.

[jkotas]

How complicated is it to get mlaunch tool launched on starting with a random phone out there?

I think we need to focus on reducing production diagnostic friction with this feature. For me, the production diagnostic means "My app is crashing on my mom's phone, I have no idea why and it does not repro for me locally". Ideally, I would tell her to do a few simple steps, let the app crash, and then get her to send me some file (e.g. crash dump with stresslog) that allows me to figure out what's wrong.

[jkotas]

Left some feedback.

I think the main question I would like to see answered is how we are going to do this on Apple devices. We should have coherent story between iOS and Android.

[lateralusX]

Left some feedback.

I think the main question I would like to see answered is how we are going to do this on Apple devices. We should have coherent story between iOS and Android.

On Apple platforms we have mlauch tool that launches that app and can set env variables using its --setenv argument, see https://github.com/dotnet/maui/wiki/Profiling-.NET-MAUI-Apps#ios-mac-etc for some examples on running EventPipe on iOS.

[risc-vv]

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.

[risc-vv]

RISC-V Release-CLR-VF2: 9079 / 9109 (99.67%)

=======================
      passed: 9079
      failed: 2
     skipped: 600
      killed: 28
------------------------
 TOTAL tests: 9709
VIRTUAL time: 12h 10min 12s 316ms
   REAL time: 49min 5s 73ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-CLR-QEMU: 9078 / 9108 (99.67%)

=======================
      passed: 9078
      failed: 2
     skipped: 600
      killed: 28
------------------------
 TOTAL tests: 9708
VIRTUAL time: 37h 33min 34s 112ms
   REAL time: 38min 22s 159ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 282093 / 283176 (99.62%)

=======================
      passed: 282093
      failed: 1074
     skipped: 39
      killed: 9
------------------------
 TOTAL tests: 283215
VIRTUAL time: 30h 54min 12s 214ms
   REAL time: 1h 10min 9s 791ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: 88c66b4b3883d92b981e15b9c82310e63e0a7366
CI: d6c9c1ab3a7411819463edc05ded301e89ba586a
REPO: lateralusX/runtime
BRANCH: lateralusX/add-android-sys-prop-support-2
CONFIG: Release
LIB_CONFIG: Release

[jkotas]

I have looked at this again. It may work better to do a cleanup PR that moves the environment handling to minipal, unifies it between CoreCLR, NativeAOT, Mono and CoreLib. And then do the environment variable injection feature for mobile platforms on top of it. It feels that converging both the minipal cleanup and the new feature together in the same PR will take a while.

Observation: The environment variable injection feature for mobile platforms really just wants to read a list of env variables overrides early during startup and apply it. It can be triggered explicitly early during runtime startup and call the lower level minipal environment APIs. It does not need to happen implicitly whenever something tries to read an environment for the first time.

[lateralusX]

I have looked at this again. It may work better to do a cleanup PR that moves the environment handling to minipal, unifies it between CoreCLR, NativeAOT, Mono and CoreLib. And then do the environment variable injection feature for mobile platforms on top of it. It feels that converging both the minipal cleanup and the new feature together in the same PR will take a while.

Observation: The environment variable injection feature for mobile platforms really just wants to read a list of env variables overrides early during startup and apply it. It can be triggered explicitly early during runtime startup and call the lower level minipal environment APIs. It does not need to happen implicitly whenever something tries to read an environment for the first time.

I believe what we currently have in this PR is close, currently it unifies the env implementation in minipal, hooks up NAOT and CoreLib to use it. CoreCLR changes are kept small to reduce regression, just sourcing with an environ that reflects all changes to current environ (on Android it get merged with system properties). You can explicitly trigger a load of env using load function or do lazy loading if needed. Going forward we can do full replace CoreCLR environ.cpp with minipal env.c, but right now it keeps its own PAL API reflecting the Windows API, going forward we could replace it with minipal implementation or redirecting its internal implementation keeping the Windows API internally for CoreCLR (what this PR did in history).

If we believe it's easier to split this PR into one PR that adds the new minipal env support and integrating it into the different runtimes, I can certainly do that, but for the first integration PR I would probably do similar strategy that we already have in this PR, minimize impact on existing CoreCLR env API to reduce regression and impact on Windows platform. Going forward we could replace internal use of CoreCLR env PAL API and reduce PAL abstraction (but will impact Windows platforms as well). Would that be acceptable?

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.