-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Don't retain host probing-path properties in CLR config and AppContext #129995
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
99e8920
1ee20af
da66f78
6dc8fd3
0f9a332
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,17 @@ class ConstWStringArrayHolder : public NewArrayHolder<LPCWSTR> | |
| } | ||
| }; | ||
|
|
||
| // Returns true for known host properties that do not need to be stored as CLR configuration. | ||
| // The runtime parses these into binder/AppDomain structures during AppDomain creation, so it | ||
| // does not need to also keep the original raw strings around in the CLR config knobs. | ||
| static bool IsKnownHostProperty(LPCSTR key) | ||
| { | ||
| return strcmp(key, HOST_PROPERTY_TRUSTED_PLATFORM_ASSEMBLIES) == 0 | ||
| || strcmp(key, HOST_PROPERTY_NATIVE_DLL_SEARCH_DIRECTORIES) == 0 | ||
| || strcmp(key, HOST_PROPERTY_PLATFORM_RESOURCE_ROOTS) == 0 | ||
| || strcmp(key, HOST_PROPERTY_APP_PATHS) == 0; | ||
| } | ||
|
|
||
| // Convert 8 bit string to unicode | ||
| static LPCWSTR StringToUnicode(LPCSTR str) | ||
| { | ||
|
|
@@ -294,8 +305,29 @@ int coreclr_initialize( | |
| Bundle::AppBundle = &bundle; | ||
| } | ||
|
|
||
| // This will take ownership of propertyKeysWTemp and propertyValuesWTemp | ||
| Configuration::InitializeConfigurationKnobs(propertyCount, propertyKeysW, propertyValuesW); | ||
| // Build a filtered set of properties for the CLR config knobs that excludes probing path | ||
| // properties (TPA, NATIVE_DLL_SEARCH_DIRECTORIES, PLATFORM_RESOURCE_ROOTS, APP_PATHS). | ||
| // Those are parsed into binder/AppDomain structures during CreateAppDomainWithManager, | ||
| // so the runtime does not need a duplicate copy held forever in the CLR config knobs. | ||
| // The TPA list in particular can be tens of KB. | ||
| LPCWSTR* configKeysW = new (nothrow) LPCWSTR[propertyCount]; | ||
| ASSERTE_ALL_BUILDS(configKeysW != nullptr); | ||
| LPCWSTR* configValuesW = new (nothrow) LPCWSTR[propertyCount]; | ||
| ASSERTE_ALL_BUILDS(configValuesW != nullptr); | ||
|
|
||
| int configPropertyCount = 0; | ||
| for (int i = 0; i < propertyCount; ++i) | ||
| { | ||
| if (IsKnownHostProperty(propertyKeys[i])) | ||
| continue; | ||
|
|
||
| configKeysW[configPropertyCount] = propertyKeysW[i]; | ||
| configValuesW[configPropertyCount] = propertyValuesW[i]; | ||
| ++configPropertyCount; | ||
| } | ||
|
|
||
| // Configuration takes ownership of the filtered arrays and the strings they reference. | ||
| Configuration::InitializeConfigurationKnobs(configPropertyCount, configKeysW, configValuesW); | ||
|
|
||
| #ifdef TARGET_UNIX | ||
| if (Configuration::GetKnobBooleanValue(W("System.Runtime.CrashReportBeforeSignalChaining"), CLRConfig::INTERNAL_CrashReportBeforeSignalChaining)) | ||
|
|
@@ -324,6 +356,21 @@ int coreclr_initialize( | |
| propertyValuesW, | ||
| (DWORD *)domainId); | ||
|
|
||
| // The binder/AppDomain has now parsed the host probing path properties into its own | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps a RAII style class that handles the allocations done in |
||
| // structures and the managed AppContext.Setup has copied each property value into a | ||
| // managed string. Free the wide-string copies for the excluded properties (the rest are | ||
| // owned by the CLR config above) and the full property arrays themselves. | ||
|
elinor-fung marked this conversation as resolved.
|
||
| for (int i = 0; i < propertyCount; ++i) | ||
| { | ||
|
elinor-fung marked this conversation as resolved.
|
||
| if (IsKnownHostProperty(propertyKeys[i])) | ||
| { | ||
| delete[] (WCHAR*)propertyKeysW[i]; | ||
| delete[] (WCHAR*)propertyValuesW[i]; | ||
| } | ||
| } | ||
| delete[] propertyKeysW; | ||
| delete[] propertyValuesW; | ||
|
Comment on lines
+370
to
+371
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make these holders now and avoid the |
||
|
|
||
| if (SUCCEEDED(hr)) | ||
| { | ||
| host.SuppressRelease(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,9 @@ | |||||
| #include "appdomain.inl" | ||||||
| #include "eventtrace.h" | ||||||
| #include "../binder/inc/defaultassemblybinder.h" | ||||||
| #include "../binder/inc/applicationcontext.hpp" | ||||||
| #include <corehost/host_runtime_contract.h> | ||||||
| #include "stringarraylist.h" | ||||||
|
|
||||||
| // static | ||||||
| extern "C" void QCALLTYPE AppDomain_CreateDynamicAssembly(QCall::ObjectHandleOnStack assemblyLoadContext, NativeAssemblyNameParts* pAssemblyNameParts, INT32 hashAlgorithm, INT32 access, QCall::ObjectHandleOnStack retAssembly) | ||||||
|
|
@@ -112,6 +115,118 @@ extern "C" void QCALLTYPE AssemblyNative_GetLoadedAssemblies(QCall::ObjectHandle | |||||
| END_QCALL; | ||||||
| } | ||||||
|
|
||||||
| namespace | ||||||
| { | ||||||
| // Append all paths from a StringArrayList into 'output', separated by PATH_SEPARATOR_CHAR_W. | ||||||
| void AppendStringArrayList(StringArrayList* pList, SString& output) | ||||||
| { | ||||||
| if (pList == NULL) | ||||||
| return; | ||||||
|
Comment on lines
+123
to
+124
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs contract. Make assert. Functions like this shouldn't normally need these defensive checks given all callers of it already check the invariant. |
||||||
|
|
||||||
| for (DWORD i = 0; i < pList->GetCount(); ++i) | ||||||
| { | ||||||
| if (i != 0) | ||||||
| output.Append(PATH_SEPARATOR_CHAR_W); | ||||||
|
|
||||||
| output.Append(pList->Get(i)); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Get the value of a known host property from the binder/AppDomain state. | ||||||
| extern "C" BOOL QCALLTYPE AppContext_TryGetHostPropertyValue(LPCWSTR name, QCall::StringHandleOnStack retValue) | ||||||
| { | ||||||
| QCALL_CONTRACT; | ||||||
|
|
||||||
| BOOL found = FALSE; | ||||||
|
|
||||||
| BEGIN_QCALL; | ||||||
|
|
||||||
| AppDomain* pDomain = AppDomain::GetCurrentDomain(); | ||||||
| DefaultAssemblyBinder* pBinder = pDomain->GetDefaultBinder(); | ||||||
| BINDER_SPACE::ApplicationContext* pAppContext = pBinder->GetAppContext(); | ||||||
|
|
||||||
| if (u16_strcmp(name, _T(HOST_PROPERTY_TRUSTED_PLATFORM_ASSEMBLIES)) == 0) | ||||||
| { | ||||||
| if (pAppContext->IsTpaListProvided()) | ||||||
| { | ||||||
| BINDER_SPACE::SimpleNameToFileNameMap* pMap = pAppContext->GetTpaList(); | ||||||
| _ASSERTE(pMap != NULL); | ||||||
|
|
||||||
| StackSString result; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This will never be sufficient for the TPA. I would just start with |
||||||
| BINDER_SPACE::SimpleNameToFileNameMap::Iterator i = pMap->Begin(); | ||||||
| BINDER_SPACE::SimpleNameToFileNameMap::Iterator end = pMap->End(); | ||||||
| while (i != end) | ||||||
| { | ||||||
| if (i->m_wszILFileName != NULL) | ||||||
| { | ||||||
| if (!result.IsEmpty()) | ||||||
| result.Append(PATH_SEPARATOR_CHAR_W); | ||||||
|
|
||||||
| result.Append(i->m_wszILFileName); | ||||||
| } | ||||||
|
|
||||||
| ++i; | ||||||
| } | ||||||
|
|
||||||
| if (!result.IsEmpty()) | ||||||
| { | ||||||
| retValue.Set(result); | ||||||
| found = TRUE; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| else if (u16_strcmp(name, _T(HOST_PROPERTY_NATIVE_DLL_SEARCH_DIRECTORIES)) == 0) | ||||||
| { | ||||||
| StackSString result; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| AppDomain::PathIterator iter = pDomain->IterateNativeDllSearchDirectories(); | ||||||
| while (iter.Next()) | ||||||
| { | ||||||
| if (!result.IsEmpty()) | ||||||
| result.Append(PATH_SEPARATOR_CHAR_W); | ||||||
|
|
||||||
| result.Append(*iter.GetPath()); | ||||||
| } | ||||||
|
|
||||||
| if (!result.IsEmpty()) | ||||||
| { | ||||||
| retValue.Set(result); | ||||||
| found = TRUE; | ||||||
| } | ||||||
| } | ||||||
| else if (u16_strcmp(name, _T(HOST_PROPERTY_PLATFORM_RESOURCE_ROOTS)) == 0) | ||||||
| { | ||||||
| StringArrayList* pList = pAppContext->GetPlatformResourceRoots(); | ||||||
| if (pList != NULL && pList->GetCount() > 0) | ||||||
| { | ||||||
| StackSString result; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| AppendStringArrayList(pList, result); | ||||||
| retValue.Set(result); | ||||||
| found = TRUE; | ||||||
| } | ||||||
| } | ||||||
| else if (u16_strcmp(name, _T(HOST_PROPERTY_APP_PATHS)) == 0) | ||||||
| { | ||||||
| StringArrayList* pList = pAppContext->GetAppPaths(); | ||||||
| if (pList != NULL && pList->GetCount() > 0) | ||||||
| { | ||||||
| StackSString result; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| AppendStringArrayList(pList, result); | ||||||
| retValue.Set(result); | ||||||
| found = TRUE; | ||||||
| } | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| // Caller is expected to only request known properties. | ||||||
| _ASSERTE(!"AppContext_TryGetHostPropertyValue called with unknown name"); | ||||||
| } | ||||||
|
|
||||||
| END_QCALL; | ||||||
|
|
||||||
| return found; | ||||||
| } | ||||||
|
|
||||||
| extern "C" void QCALLTYPE String_IsInterned(QCall::StringHandleOnStack str) | ||||||
| { | ||||||
| QCALL_CONTRACT; | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.