Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions dev/Common/WindowsAppRuntimeAutoInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ class AutoInitialize
[global::System.Runtime.CompilerServices.ModuleInitializer]
internal static void InitializeWindowsAppSDK()
{
// HybridDeploy: set env vars BEFORE Bootstrap so that:
// 1. Bootstrap can detect hybrid mode and keep the package graph empty
// 2. On Win10, catalog loading during DllMain can expand loadFrom env vars
#if MICROSOFT_WINDOWSAPPSDK_AUTOINITIALIZE_HYBRIDDEPLOYSETUP
global::System.Environment.SetEnvironmentVariable("MICROSOFT_WINDOWSAPPRUNTIME_HYBRID_DEPLOY", "1");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

env vars should be avoided if at all possible. MICROSOFT_WINDOWSAPPRUNTIME_FRAMEWORK_PATH and MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY are required for loadFrom. But the boolean MICROSOFT_WINDOWSAPPRUNTIME_HYBRID_DEPLOY could instead be an API that's explicitly set by the autoinitialization source. Or better yet, can we just infer it from the existing and differing values of MICROSOFT_WINDOWSAPPRUNTIME_FRAMEWORK_PATH and MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

env vars should be avoided if at all possible

+1000

We should be seriously examining dev\UndockedRegFreeWinRT\UndockedRegFreeWinRT-AutoInitializer.cs to see how we can remove

// Set base directory env var for PublishSingleFile support (referenced by SxS redirection)
Environment.SetEnvironmentVariable("MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", AppContext.BaseDirectory);

not lean in even more heavily on it.

Q: If we need altered Assembly.Load*() behavior why aren't we adding a Custom Load Context to .NET callers? We're already adding *autoiniitalizer*.cs to .NET projects through build magic. What's a little more code?

global::System.Environment.SetEnvironmentVariable("MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", global::System.AppContext.BaseDirectory);
#endif

// Call the AutoInitialize functions, as needed, starting with those initializing the WindowsAppRuntime
#if MICROSOFT_WINDOWSAPPSDK_AUTOINITIALIZE_BOOTSTRAP
Microsoft.Windows.ApplicationModel.DynamicDependency.BootstrapCS.AutoInitialize.AccessWindowsAppSDK();
Expand Down
170 changes: 131 additions & 39 deletions dev/WindowsAppRuntime_BootstrapDLL/MddBootstrap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,52 @@

#include <filesystem>

static bool IsHybridDeploy() noexcept
{
wchar_t value[2]{};
return GetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_HYBRID_DEPLOY", value, ARRAYSIZE(value)) > 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SET MICROSOFT_WINDOWSAPPRUNTIME_HYBRID_DEPLOY=1
RunWhatever.exe

How is this not a massive DOS attack waiting to happen?

}

static void SetFrameworkPathEnvironmentVariable(PCWSTR frameworkPath)
{
SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_FRAMEWORK_PATH", frameworkPath);
}

// For C++ apps (no C# auto-initializer), set BASE_DIRECTORY to exe dir if not already set
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?
Why is it needed by C++ but not C#?
Does Rust need it?
Does Python?
Does Node?
...

MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY was added in 1.0? as an emergency workaround. Has it ever been spec'd? 100line comment somewhere?

static void SetBaseDirectoryEnvironmentVariableIfNotSet()
{
wchar_t existing[2]{};
if (GetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", existing, ARRAYSIZE(existing)) > 0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very loose in behavior

SET MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY=1
SET MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY=0
SET MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY=x
SET MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY=hello
...

But hard to assess how problematic as I can't find docs or spec about MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY.

P.S. For an example how to deal with this sort of hackery in a more rigorous fashion see IsLifetimeManagerViaEnumeration() in dev\WindowsAppRuntime_BootstrapDLL\MddBootstrap.cpp

{
return; // Already set by C# auto-initializer
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the C# auto-initializer handles this for C#
why can't/shouldn't the C++ auto-initializer handle it for C++?

}
wchar_t exePath[MAX_PATH]{};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not LongPath friendly

if (GetModuleFileNameW(nullptr, exePath, ARRAYSIZE(exePath)) > 0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wil::GetModuleFileNameW() beats raw GetModuleFileNameW()

You can find uses in winappsdk e.g. dev\WindowsAppRuntime_BootstrapDLL\MddBootstrap.cpp line 1274 in MddBootstrapInitialize_ShowUI_OnNoMatch()

{
// Extract directory from exe path and ensure trailing backslash
auto exeDir{ std::filesystem::path(exePath).parent_path().wstring() };
if (!exeDir.empty() && exeDir.back() != L'\\')
{
exeDir += L'\\';
}
SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", exeDir.c_str());
}
}

// Resolve framework package install path from package full name
static std::wstring GetFrameworkPackagePath(PCWSTR packageFullName)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to just use GetInstallPath<std::wstring>(pkgfullname) from #6200

That's only 2 steps away from checkin

  • Address Letao's codereview feedback
  • Resolve the RS5 test failure

{
UINT32 pathLength{ 0 };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: UINT32 pathLength{};

const auto rc{ GetPackagePathByFullName(packageFullName, &pathLength, nullptr) };
if (rc != ERROR_INSUFFICIENT_BUFFER)
{
THROW_WIN32(rc);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: THROW_HR_IF(HRESULT_FROM_WIN32(rc), rc != ERROR_INSUFFICIENT_BUFFER);

auto path{ std::make_unique<WCHAR[]>(pathLength) };
THROW_IF_WIN32_ERROR(GetPackagePathByFullName(packageFullName, &pathLength, path.get()));
return std::wstring(path.get());
}

HRESULT _MddBootstrapInitialize(
UINT32 majorMinorVersion,
PCWSTR versionTag,
Expand Down Expand Up @@ -417,43 +463,75 @@ void FirstTimeInitialization(
const UINT32 minVersionToUseWin11Support{ 0x00010007 };
if ((majorMinorVersion >= minVersionToUseWin11Support) && MddCore::Win11::IsSupported())
{
// Add the framework package to the package graph
const std::wstring frameworkPackageFamilyName{ GetFrameworkPackageFamilyName(majorMinorVersion, packageVersionTag.c_str()) };
const MddPackageDependencyProcessorArchitectures architectureFilter{};
const auto lifetimeKind{ MddPackageDependencyLifetimeKind::Process };
const MddCreatePackageDependencyOptions createOptions{};
wil::unique_process_heap_string packageDependencyId;
THROW_IF_FAILED(MddCore::Win11::TryCreatePackageDependency(nullptr, frameworkPackageFamilyName.c_str(), minVersion, architectureFilter, lifetimeKind, nullptr, createOptions, &packageDependencyId));
//
const MddAddPackageDependencyOptions addOptions{};
MDD_PACKAGEDEPENDENCY_CONTEXT packageDependencyContext{};
wil::unique_process_heap_string packageFullName;
THROW_IF_FAILED(MddCore::Win11::AddPackageDependency(packageDependencyId.get(), MDD_PACKAGE_DEPENDENCY_RANK_DEFAULT, addOptions, &packageDependencyContext, &packageFullName));

// Update the activity context
auto& activityContext{ WindowsAppRuntime::MddBootstrap::Activity::Context::Get() };
activityContext.SetInitializationPackageFullName(packageFullName.get());

// Pass along test information (if necessary)
if (!g_test_frameworkPackageNamePrefix.empty())
const bool hybridDeploy{ IsHybridDeploy() };
if (hybridDeploy)
{
FAIL_FAST_HR_IF(E_UNEXPECTED, g_test_ddlmPackageNamePrefix.empty());
FAIL_FAST_HR_IF(E_UNEXPECTED, g_test_ddlmPackagePublisherId.empty());
FAIL_FAST_HR_IF(E_UNEXPECTED, g_test_mainPackageNamePrefix.empty());

::WindowsAppRuntime::VersionInfo::TestInitialize(frameworkPackageFamilyName.c_str());
// HybridDeploy: temporarily add to package graph to resolve the framework path,
// then immediately remove so that the package graph stays empty.
// An empty package graph lets the SxS manifest loadFrom control DLL loading.
const MddAddPackageDependencyOptions addOptions{};
MDD_PACKAGEDEPENDENCY_CONTEXT tempContext{};
wil::unique_process_heap_string packageFullName;
THROW_IF_FAILED(MddCore::Win11::AddPackageDependency(packageDependencyId.get(), MDD_PACKAGE_DEPENDENCY_RANK_DEFAULT, addOptions, &tempContext, &packageFullName));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MddCore::Win11::* are calls to the OS DynDep APIs

I don't understand the intent. It looks like it's playing hide'n'seek with OS DynDep and winappsdk OS-alternative polyfill hacks (SetFrameworkPathEnvironmentVariable, AddDllDirectory, etc).

This is all terribly confusing and disturbing, unless I'm missing something.


const auto frameworkPath{ GetFrameworkPackagePath(packageFullName.get()) };

// Remove from package graph immediately — keep it empty for manifest loadFrom
MddCore::Win11::RemovePackageDependency(tempContext);

SetFrameworkPathEnvironmentVariable(frameworkPath.c_str());
AddDllDirectory(frameworkPath.c_str());

// Update the activity context
auto& activityContext{ WindowsAppRuntime::MddBootstrap::Activity::Context::Get() };
activityContext.SetInitializationPackageFullName(packageFullName.get());

// Track our initialized state (keep packageDependencyId alive for lifetime protection)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say whaaat? Track it...but we just removed it (line 487)

g_usingWin11Support = UsingWin11Support::Yes;
g_packageDependencyId = std::move(packageDependencyId);
g_initializationMajorMinorVersion = majorMinorVersion;
g_initializationVersionTag = std::move(packageVersionTag);
const auto frameworkPackageIdentity{ ::AppModel::Identity::PackageIdentity::FromPackageFullName(packageFullName.get()) };
g_initializationFrameworkPackageVersion.Version = frameworkPackageIdentity.Version().Version;
}
else
{
// Normal path: add the framework package to the package graph
const MddAddPackageDependencyOptions addOptions{};
MDD_PACKAGEDEPENDENCY_CONTEXT packageDependencyContext{};
wil::unique_process_heap_string packageFullName;
THROW_IF_FAILED(MddCore::Win11::AddPackageDependency(packageDependencyId.get(), MDD_PACKAGE_DEPENDENCY_RANK_DEFAULT, addOptions, &packageDependencyContext, &packageFullName));

// Update the activity context
auto& activityContext{ WindowsAppRuntime::MddBootstrap::Activity::Context::Get() };
activityContext.SetInitializationPackageFullName(packageFullName.get());

// Pass along test information (if necessary)
if (!g_test_frameworkPackageNamePrefix.empty())
{
FAIL_FAST_HR_IF(E_UNEXPECTED, g_test_ddlmPackageNamePrefix.empty());
FAIL_FAST_HR_IF(E_UNEXPECTED, g_test_ddlmPackagePublisherId.empty());
FAIL_FAST_HR_IF(E_UNEXPECTED, g_test_mainPackageNamePrefix.empty());

// Track our initialized state
g_usingWin11Support = UsingWin11Support::Yes;
//
g_packageDependencyId = std::move(packageDependencyId);
g_packageDependencyContext = packageDependencyContext;
//
g_initializationMajorMinorVersion = majorMinorVersion;
g_initializationVersionTag = std::move(packageVersionTag);
const auto frameworkPackageIdentity{ ::AppModel::Identity::PackageIdentity::FromPackageFullName(packageFullName.get()) };
g_initializationFrameworkPackageVersion.Version = frameworkPackageIdentity.Version().Version;
::WindowsAppRuntime::VersionInfo::TestInitialize(frameworkPackageFamilyName.c_str());
}

// Track our initialized state
g_usingWin11Support = UsingWin11Support::Yes;
g_packageDependencyId = std::move(packageDependencyId);
g_packageDependencyContext = packageDependencyContext;
g_initializationMajorMinorVersion = majorMinorVersion;
g_initializationVersionTag = std::move(packageVersionTag);
const auto frameworkPackageIdentity{ ::AppModel::Identity::PackageIdentity::FromPackageFullName(packageFullName.get()) };
g_initializationFrameworkPackageVersion.Version = frameworkPackageIdentity.Version().Version;
}
}
else
{
Expand All @@ -469,6 +547,15 @@ void FirstTimeInitialization(
// Temporarily add the framework's package directory to PATH so LoadLibrary can find it and any colocated imports
wil::unique_dll_directory_cookie dllDirectoryCookie{ AddFrameworkToPath(frameworkPackageInfo->path) };

const bool hybridDeploy{ IsHybridDeploy() };
if (hybridDeploy)
{
// HybridDeploy: set env vars BEFORE loading the DLL so that
// catalog.cpp can expand %FRAMEWORK_PATH% and %BASE_DIRECTORY% in loadFrom
SetFrameworkPathEnvironmentVariable(frameworkPackageInfo->path);
SetBaseDirectoryEnvironmentVariableIfNotSet();
}

auto windowsAppRuntimeDllFilename{ std::wstring(frameworkPackageInfo->path) + L"\\Microsoft.WindowsAppRuntime.dll" };
wil::unique_hmodule windowsAppRuntimeDll(LoadLibraryEx(windowsAppRuntimeDllFilename.c_str(), nullptr, LOAD_WITH_ALTERED_SEARCH_PATH));
if (!windowsAppRuntimeDll)
Expand All @@ -477,20 +564,25 @@ void FirstTimeInitialization(
THROW_WIN32_MSG(lastError, "Error in LoadLibrary: %d (0x%X) loading %ls", lastError, lastError, windowsAppRuntimeDllFilename.c_str());
}

// Add the framework package to the package graph
const MddPackageDependencyProcessorArchitectures architectureFilter{};
const auto lifetimeKind{ MddPackageDependencyLifetimeKind::Process };
const MddCreatePackageDependencyOptions createOptions{};
wil::unique_process_heap_string packageDependencyId;
THROW_IF_FAILED(MddTryCreatePackageDependency(nullptr, frameworkPackageInfo->packageFamilyName, minVersion, architectureFilter, lifetimeKind, nullptr, createOptions, &packageDependencyId));
//
const MddAddPackageDependencyOptions addOptions{};
MDD_PACKAGEDEPENDENCY_CONTEXT packageDependencyContext{};
THROW_IF_FAILED(MddAddPackageDependency(packageDependencyId.get(), MDD_PACKAGE_DEPENDENCY_RANK_DEFAULT, addOptions, &packageDependencyContext, nullptr));

// Remove our temporary path addition
RemoveFrameworkFromPath(frameworkPackageInfo->path);
dllDirectoryCookie.reset();
if (!hybridDeploy)
{
// Normal path: add the framework package to the package graph
const MddPackageDependencyProcessorArchitectures architectureFilter{};
const auto lifetimeKind{ MddPackageDependencyLifetimeKind::Process };
const MddCreatePackageDependencyOptions createOptions{};
THROW_IF_FAILED(MddTryCreatePackageDependency(nullptr, frameworkPackageInfo->packageFamilyName, minVersion, architectureFilter, lifetimeKind, nullptr, createOptions, &packageDependencyId));
//
const MddAddPackageDependencyOptions addOptions{};
THROW_IF_FAILED(MddAddPackageDependency(packageDependencyId.get(), MDD_PACKAGE_DEPENDENCY_RANK_DEFAULT, addOptions, &packageDependencyContext, nullptr));

// Remove our temporary path addition (package graph handles DLL search now)
RemoveFrameworkFromPath(frameworkPackageInfo->path);
dllDirectoryCookie.reset();
}
// else: HybridDeploy keeps the framework path in DLL search order
// since there's no package graph to handle it

// Pass along test information (if necessary)
if (!g_test_ddlmPackageNamePrefix.empty())
Expand Down