search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.17k stars 4.72k forks source link

BaseBucketParamsManager::GetAppVersion should switch to PREEMPT before calling DwGetFileVersionInfo #35866

Closed Maoni0 closed 4 years ago

Maoni0 commented 4 years ago

I'm observing this code from EH is preventing suspension from completing in a timely manner -

+ coreclr!IL_Throw
+ coreclr!RaiseTheExceptionInternalOnly
+ kernelbase!RaiseException
+ ntdll!RtlRaiseException
+ ntdll!RtlDispatchException
+ ntdll!RtlpExecuteHandlerForException
+ coreclr!__C_specific_handler
+ coreclr!CallDescrWorkerReflectionWrapper'::1'::filt$0
+ coreclr!ReflectionInvocationExceptionFilter
+ coreclr!SetupWatsonBucketsForNonPreallocatedExceptions
+ coreclr!GetBucketParametersForManagedException
+ coreclr!GetManagedBucketParametersForIp
+ coreclr!CLR20r3BucketParamsManager::PopulateBucketParameters
+ coreclr!BaseBucketParamsManager::GetAppVersion
+ coreclr!DwGetFileVersionInfo
+ coreclr!GetFileVersion
+ coreclr!GetFileVersionInfoSizeW_NoThrow
+ kernelbase!GetFileVersionInfoSizeExW
+ kernelbase!LoadLibraryExW
+ kernelbase!BasepLoadLibraryAsDataFileInternal
+ ntdll!RtlDosSearchPath_Ustr
+ ntdll!RtlDoesFileExists_UstrEx
+ ntdll!ZwQueryAttributesFile
+ ntoskrnl!NtQueryAttributesFile
+ ntoskrnl!ObOpenObjectByNameEx
+ ntoskrnl!ObpLookupObjectName
+ ntoskrnl!IopParseDevice
+ ntoskrnl!IopQueryInformation
+ fltmgr!FltpFastIoQueryOpen
+ fltmgr!FltpPerformFastIoCall
+ ntfs!NtfsNetworkOpenCreate
+ ntfs!NtfsCommonQueryOpen
+ ntfs!NtfsCommonCreate
+ ntfs!NtfsAcquireSharedVcb
+ ntoskrnl!ExAcquireResourceSharedLite
+ ntoskrnl!ExpAcquireResourceSharedLite
+ ntoskrnl!ExpWaitForResource
+ ntoskrnl!KeWaitForSingleObject
+ ntoskrnl!KiCommitThreadWait
+ ntoskrnl!KiSwapThread
+ ntoskrnl!KiDeliverApc
+ ntoskrnl!PspGetSetContextSpecialApc
+ ntoskrnl!KeSignalGate
+ ntoskrnl!KiExitDispatcher
+ EventData Readied Thread 7736 Proc 21104
+ Event Windows Kernel/Dispatcher/ReadyThread

or DwGetFileVersionInfo could do that before calling GetFileVersion. this is causing suspension that's 3 to 4s long.

Dotnet-GitSync-Bot commented 4 years ago

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

mangod9 commented 4 years ago

@Maoni0 I have a presumptive fix here: https://github.com/dotnet/runtime/pull/36151. Though from what I can see the watson codepath should only be hit in unhandled exception scenarios?

Maoni0 commented 4 years ago

chatted with some folks who are more familiar with EH about this. the gist is there may be code paths that handle exceptions that the runtime may not know for sure are unhandled (so they may or may not be unhandled later), at least this was the case on desktop. also this does hit a particular EH which is ReflectionInvocationExceptionFilter. the process clearly kept on running for the case I looked at.

mangod9 commented 4 years ago
mangod9 commented 4 years ago

Ok I was able to hit the watson codepath with the reflection invocation exception case. Looks like ReflectionInvocationExceptionFilter switches to COOP just before calling SetupWatsonBucketsForNonPreallocatedExceptions: https://github.com/dotnet/runtime/blob/e77572ffccc566186f47207f3c5475533c87538e/src/coreclr/src/vm/excep.cpp#L8441

Assume that is required to gcprotect oThrowable?

mangod9 commented 4 years ago

fix has been merged