-
Notifications
You must be signed in to change notification settings - Fork 528
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
Blazor hang with marshal methods #7927
Conversation
* main: [Xamarin.Android.Build.Tasks] Fix Android Version Code for Release builds (dotnet#7795)
Better string handling in LLVM IR generator
@@ -43,6 +46,10 @@ sealed class InputFiles | |||
[Required] | |||
public string AndroidBinUtilsDirectory { get; set; } | |||
|
|||
public string AndroidNdkDirectory { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does EnableMarshalMethodTracing
require the NDK, and not just the clang build tools we already redistribute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, it's to find libunwind.a
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not only libunwind.a
, also a few others from the compiler "private" collection (libc++abi.a
, libclang_rt.builtins-<ARCH>-android.a
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracing no longer requires NDK to be present on the end user's machine
@@ -171,6 +171,9 @@ sealed class MarshalMethodName | |||
{ 'L', typeof(_jobjectArray) }, | |||
}; | |||
|
|||
const string mm_trace_func_enter_name = "_mm_trace_func_enter"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems kinda odd that most of the members here are camelCase or PascalCase, but then the new members are c_snake_case
. Should this not be MMTraceFuncEnterName
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a standalone C/C++ function, so I used the C/C++ naming convention.
@@ -2085,6 +2087,8 @@ because xbuild doesn't support framework reference assemblies. | |||
ApplicationSharedLibraries="@(_ApplicationSharedLibrary)" | |||
DebugBuild="$(AndroidIncludeDebugSymbols)" | |||
AndroidBinUtilsDirectory="$(AndroidBinUtilsDirectory)" | |||
EnableMarshalMethodTracing="$(_AndroidEnableMarshalMethodTracing)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when $(_AndroidEnableMarshalMethodTracing)
=True and $(_AndroidNdkDirectory)
isn't set and/or is the "internal" clang tooling? What's the error message look like? Is that error message acceptable?
(Sure, this is a private property for now, but even we make mistakes…)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now it's ArgumentNullException(nameof (ndk))
in the method where we actually use the NDK. Not sure if we ever want to make this property "public", though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer a problem, NDK not required anymore
return mm_class_names[class_index]; | ||
} | ||
|
||
static uint64_t get_method_id (uint32_t mono_image_index, uint32_t method_token) noexcept |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why "pack" these values into a uint64_t
instead of a std::tuple<uint32_t, uint32_t>
or some other struct
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a constant, faster to pass (no pointer, less memory access), can easily be used as a selector when looking for method name/description/etc in an array, and takes up less LLVM IR code (easier to read and understand, not to mention generate):
tail call void @_mm_trace_func_enter (i32 126, i32 0, i32 100664310, i8* getelementptr inbounds ([137 x i8], [137 x i8]* @__str.0, i64 0, i64 0)) #2
tail call void %get_func_ptr (i32 126, i32 0, i32 100664310, i8** nonnull align 8 dereferenceable(8) bitcast (void (%class._JNIEnv*, %class._jclass*, %class._jobject*, %class._jobject*, i32, i64)** @native_cb_onItemClick_126_0_60003f6 to i8**)) #2
if it were a pointer to a struct (or tuple) we'd need to output the struct declaration, variable definition, use a pointer which would be another ugly getelementptr
invocation. It would make both calls, and especially the 2nd one, even uglier and less readable.
* main: [Mono.Android] Bind API-UpsideDownCake Developer Preview 1 (dotnet#7796) Bump to dotnet/installer@d109cba3ff 8.0.100-preview.4.23176.5 (dotnet#7921)
Use standalone libunwind instead of the LLVM's Instead of using autoconf on *nix and cmake on Windows, provide our own CMakeLists.txt file which works everywhere we need.
Building application with tracing currently fails because of __cxa_demangle missing symbols (it requires the built-in clang libunwind which we cannot use) - we will need our own demangler. Next week.
* main: [tests] Remove `net472` multitargeting (dotnet#7932) [monodroid] Fix `ld` build error on Nightly Builds. (dotnet#7925) Bump to xamarin/Java.Interop/main@0355acf (dotnet#7931) [tests] Use msftconnecttest.com in QuoteInvalidQuoteUrlsShouldWork (dotnet#7919) [ci] Don't set demands for megapipeline (dotnet#7929)
* provide library stubs for liblog and libdl * include libc++abi demangler directly in the trace library (its sources are part of the NDK) * better trace output The only remaining dependency on NDK is the clang builtins library (compiler-rt). Should be able to eliminate that too, with some tweaks.
At XA build time we find and package the compiler's builtins libraries for all the ABIs we support. This allows us to link libxamarin-app.so without requiring NDK to be present on end user's machine. Additionally, traces have now adjusted addresses and offsets for all frames, to match bionic behavior. Finally, the entire trace output is logged in a single message. This makes everything faster, but also makes sure that the trace won't be interleaved with unrelated messages in logcat.
* main: [readme] Add aka.ms links for d17.5. (dotnet#7935)
Also: introduce tracing modes, 'basic` for just the method enter/leave messages and 'full' to include native and java stack traces on method entry.
* main: [ci] Remove remaining Classic test jobs. (dotnet#7924) Add Nightly Tests for Humanizer
* main: [ci] Stop building classic test suites. (dotnet#7938)
It appears that the following might be cause of the "hang": 04-07 19:39:36.121 4859 4859 I chromium: [INFO:CONSOLE(2)] "Uncaught ReferenceError: Blazor is not defined", source: https://0.0.0.0/ (2) It is probably caused by this JavaScript fragment from Blazor: Blazor.start(); window.__BlazorStarted = true; And the Blazor class might not be initialized because of a problem with delegates using the "old" registration mechanism when marshal methods are enabled (there are quite a few of them in the test app). lldb didn't reveal anything interesting, process state appeared to be fine.
* main: [Xamarin.Android] Remove OpenTK, sqlite-xamarin, System.EnterpriseServices. (dotnet#7940)
* main: Bump to xamarin/Java.Interop/main@a172402 (dotnet#7944)
// TODO: error handling | ||
trace.append ("\n Java stack trace:"); | ||
jobject current_thread = env->CallStaticObjectMethod (java_lang_Thread, java_lang_Thread_currentThread); | ||
auto stack_trace_array = static_cast<jobjectArray>(env->CallNonvirtualObjectMethod (current_thread, java_lang_Thread, java_lang_Thread_getStackTrace)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be CallObjectMethod
, as java.lang.Thread.getStackTrace()
is a virtual (non-final
) method.
The same library will be used by marshal methods tracing (by linking it into `libxamarin-app.so` when tracing is enabled) or will be used by `libmonodroid.so` (or p/invoked from managed land) if desired.
* main: Bump com.android.tools:r8 from 4.0.52 to 8.0.40 (dotnet#7934)
* main: [Xamarin.Android.Build.Tasks] fix cases of missing `@(Reference)` (dotnet#7947) Bumping to the correct monodroid commit Trying to bump monodroid to run debugger-tests Pass timeout to runtime
Eventually to be used, optionally, by libmonodroid
* main: Convert `/tools` and `/build-tools` projects from `net472` to `$(DotNetStableTargetFramework)` (dotnet#7943)
The DSO is packaged only if marshal methods tracing is enabled or if native stack traces are enabled (via the `_UseNativeStackTraces` MSBuild property) Added a p/invoke which can log any combination of: native, java, managed stack traces as well as the state of signal handlers. Updated p/invoke dispatch tables
* main: Bump to xamarin/Java.Interop/main@554d819 (dotnet#7951) [Microsoft.Android.Sdk.ILLink] fix crash when TZ changes (dotnet#7956) [tests] Port 'Xamarin.Android.JcwGen-Tests.JcwGen-Tests' to .NET (dotnet#7949) [Xamarin.Android.Build.Tasks] remove `pdb2mdb` (dotnet#7950) [ci] Add some extra params to configure the test templates (dotnet#7955)
* main: [Xamarin.Android.Build.Tasks] <GenerateJavaStubs/> should open readonly in some cases (dotnet#8129) [Mono.Android] suppress/solve more illink warnings (dotnet#8063)
First two instructions implemented (`store` and `ret`), together with support for TBAA metadata (https://llvm.org/docs/LangRef.html#tbaa-metadata) `xamarin_app_init` now fully generated.
* main: Bump to Tessil/robin-map@784245b4 [v1.2.1] (dotnet#8128)
* main: [Xamarin.Android.Build.Tasks] Allow override of `uncompressedGlob` (dotnet#7965)
* main: Bump to dotnet/installer@8d98e5a6ba 8.0.100-preview.6.23318.1 (dotnet#8131)
Tomorrow, testing and cleanup
* main: [One .NET] fix 'dotnet publish' with no arguments (dotnet#8137) [build] consider `$NUGET_PACKAGES` for `$(XAPackagesDir)` (dotnet#8136) Bump external/xamarin-android-tools from `44885bc` to `3cee10b` (dotnet#8121)
* main: [tests] Remove `XASdkDeployTests` (dotnet#8139)
* main: Bump to google/bundletool@f17ce94a (dotnet#8135) [Xamarin.Android.Build.Tasks] Handle IOException in Aapt2Daemon (dotnet#8130) [tests] don't set `/uses-sdk@android:targetSdkVersion=34` by default (dotnet#8138)
* main: [Mono.Android] Bind and enumify API-34 (dotnet#8116)
* main: $(AndroidPackVersionSuffix)=preview.7; net8 is 34.0.0-preview.7 (dotnet#8149)
* main: [Xamarin.Android.Build.Tasks] MarshalMethodsAssemblyRewriter+new file (dotnet#8151) Bump to dotnet/installer@d2a244f560 8.0.100-preview.7.23325.5 (dotnet#8142)
* main: (306 commits) [templates] Remove redundant "template" from display name. (dotnet#8773) Bump to xamarin/Java.Interop/main@a7e09b7 (dotnet#8793) [build] Include MIT license in most NuGet packages (dotnet#8787) Bump to dotnet/installer@893b762b6e 9.0.100-preview.3.24153.2 (dotnet#8782) [docs] update notes about `dotnet-trace` and `dotnet-gcdump` (dotnet#8713) [Mono.Android] Fix race condition in AndroidMessageHandler (dotnet#8753) [ci] Fix SDL Sources Analysis for PRs from forks (dotnet#8785) [ci] Add 1ESPT override to MSBuild test stages (dotnet#8784) [ci] Do not use @self annotation for templates (dotnet#8783) [ci] Migrate to the 1ES template (dotnet#8747) [Mono.Android] fix trimming warnings, part 2 (dotnet#8758) [Xamarin.Android.Build.Tasks] set `%(DefineConstantsOnly)` for older API levels (dotnet#8777) [tests] fix duplicate sources in `NuGet.config` (dotnet#8772) Bump to xamarin/monodroid@e13723e701 (dotnet#8771) Bump to xamarin/xamarin-android-tools/main@37d79c9 (dotnet#8752) Bump to dotnet/installer@d070660282 9.0.100-preview.3.24126.2 (dotnet#8763) Bump to xamarin/java.interop/main@14a9470 (dotnet#8766) $(AndroidPackVersionSuffix)=preview.3; net9 is 34.99.0.preview.3 (dotnet#8765) [Mono.Android] Do not dispose request content stream in AndroidMessageHandler (dotnet#8764) Bump com.android.tools:r8 from 8.2.42 to 8.2.47 (dotnet#8761) ...
Migrated to #8800 |
NOTE: LLVM IR generator updates are now done in #8140
Context: https://github.com/aspnet/AspNetCore-ManualTests/issues/1934