Fix [SupportedOSPlatform] warnings in Mono.Android.dll
#7590
Assignees
Labels
Area: Mono.Android
Issues with the Android API binding (Mono.Android.dll).
Milestone
Comments
jpobst
added
the
Area: Mono.Android
ghost
added
the
|
Context: 7004467 |
jonpryor
pushed a commit
to dotnet/java-interop
that referenced
this issue
Jan 26, 2023
Context: dotnet/android#7590 Context: dotnet/android@7004467 While building `Mono.Android.dll`, we had 1462 `CA1422` warnings of the following caused by an interface being marked as `[ObsoleteOSPlatform]` but the interface invoker class is not similarly annotated: …\xamarin-android\src\Mono.Android\obj\Debug\net8.0\android-33\mcw\Org.Apache.Commons.Logging.ILog.cs(128,11): warning CA1422: This call site is reachable on: 'Android' 21.0 and later. 'ILog' is obsoleted on: 'Android' 22.0 and later (This class is obsoleted in this android platform). There were also 42 `CA1422` warnings caused by interface async extension classes not having `[ObsoleteOSPlatform]` attributes: …\xamarin-android\src\Mono.Android\obj\Debug\net8.0\android-33\mcw\Org.Apache.Http.IO.ISessionOutputBuffer.cs(52,58): warning CA1422: This call site is reachable on: 'Android' 21.0 and later. 'ISessionOutputBuffer.Write(byte[]?, int, int)' is obsoleted on: 'Android' 22.0 and later (This class is obsoleted in this android platform). These warnings were disabled in dotnet/android@70044670. Fix these warnings by adding the `[ObsoleteOSPlatform]` attribute to these types if the source interface is deprecated.
jonpryor
pushed a commit
that referenced
this issue
Aug 10, 2023
Fixes: #7590 Fix the remaining `[SupportedOSPlatform]` warnings in manually bound code. Other issues we were previously seeing appear to have been a bug in earlier .NET 8 previews. In the simple cases, we just needed to add `[SupportedOSPlatform]` to our hand-written methods. For more complicated cases, we resolve the remaining CA1416 and CA1422 warnings by using `[SuppressMessage]`. Rationale: The constructors for [`java.lang.Boolean(boolean)`][0], [`java.lang.Integer(int)`][2], etc., were marked as `@Deprecated` in API-33, with a suggestion to instead use [`Booleal.valueOf(boolean)`][1], [`Integer.valueOf(int)`][3], etc. We *cannot* follow this advice in multiple places of our infrastructure, due to a desire to reduce JNI Global Reference (GREF) usage and preserve thread safety. Imagine that two threads execute the following code: using (var val = Java.Lang.Integer.ValueOf(42)) { val.FloatValue(); } Because we attempt to preserve object identity -- if multiple Java calls of `Integer.valueOf(42)` return the same instance, then multiple C# calls of `Integer.ValueOf(42)` should likewise return the same instance -- we may be dealing with mutable shared state! 1. Thread 1: calls C# `Integer.ValueOf(42)`, calls Java `Integer.valueOf(42)`, `Java.Lang.Integer` wrapper instance created and registered. 2. Thread 1: calls `val.FloatValue()` 3. Thread 2: calls `Integer.ValueOf(42)`, gets wrapper instance created in (1) 4. Thread 1: calls `val.Dispose()` 5. Thread 2: calls `val.FloatValue()`. This throws an `ObjectDisposedException`, because of (4). We thus have a conundrum: if we replace all usage of the e.g. `Integer(int)` constructor with calls to `Integer.ValueOf(int)`, *we can no longer* call `.Dispose()` on the value. This willl increase GREF usage, *but could be an overall improvement* as it may also result in fewer Java-side `Integer` instances being created. Without additional testing and microbenchmarks, it's hard to say which is actually "better". In the absense of such a micobenchmark, resolve some of the CA1416 and CA1422 warnings by using `[SuppressMessage]`. [0]: https://developer.android.com/reference/java/lang/Boolean#Boolean(boolean) [1]: https://developer.android.com/reference/java/lang/Boolean#valueOf(boolean) [2]: https://developer.android.com/reference/java/lang/Integer#Integer(int) [3]: https://developer.android.com/reference/java/lang/Integer#valueOf(int)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The .NET builds of
Mono.Android.dllcontain ~3K warnings apiece for[SupportedOSPlatform]issues:We need to investigate these and see:
CA1416: ... is only supported on: .seems a little fishy, are we doing something incorrectlyThis will likely be a long term project done over the course of the .NET 8 timeframe.
However, outputting all these warnings are currently slowing down our builds, so we should suppress them for now with
<NoWarn>. This issue is a reminder that we need to investigate this further and try for a better fix.The text was updated successfully, but these errors were encountered: