From b152761dfddc0a131dcd13f70ae0e9b9e41b37fe Mon Sep 17 00:00:00 2001
From: Sean Hall <r.sean.hall@gmail.com>
Date: Mon, 31 Jan 2022 15:44:20 -0600
Subject: Remove orphan package providers when unregistering the bundle.

Fixes #3850
---
 src/burn/engine/dependency.cpp                     | 61 +++++++++++++++++++---
 src/burn/engine/registration.cpp                   |  8 +--
 src/ext/Dependency/ca/wixdepca.cpp                 |  4 ++
 .../burn/WixToolsetTest.BurnE2E/DependencyTests.cs |  2 +-
 4 files changed, 60 insertions(+), 15 deletions(-)

(limited to 'src')

diff --git a/src/burn/engine/dependency.cpp b/src/burn/engine/dependency.cpp
index ce183a8a..e94b0e93 100644
--- a/src/burn/engine/dependency.cpp
+++ b/src/burn/engine/dependency.cpp
@@ -77,6 +77,9 @@ static void UnregisterPackageDependency(
     __in const BURN_PACKAGE* pPackage,
     __in_z LPCWSTR wzDependentProviderKey
     );
+static void UnregisterOrphanPackageProviders(
+    __in const BURN_PACKAGE* pPackage
+    );
 
 
 // functions
@@ -734,15 +737,19 @@ extern "C" void DependencyUnregisterBundle(
     HRESULT hr = S_OK;
     LPCWSTR wzDependentProviderKey = pRegistration->sczId;
 
-    // Remove the bundle provider key.
-    hr = DepUnregisterDependency(pRegistration->hkRoot, pRegistration->sczProviderKey);
-    if (SUCCEEDED(hr))
-    {
-        LogId(REPORT_VERBOSE, MSG_DEPENDENCY_BUNDLE_UNREGISTERED, pRegistration->sczProviderKey);
-    }
-    else if (FAILED(hr) && E_FILENOTFOUND != hr)
+    // If we own the bundle dependency then remove it.
+    if (!pRegistration->fDetectedForeignProviderKeyBundleId)
     {
-        LogId(REPORT_VERBOSE, MSG_DEPENDENCY_BUNDLE_UNREGISTERED_FAILED, pRegistration->sczProviderKey, hr);
+        // Remove the bundle provider key.
+        hr = DepUnregisterDependency(pRegistration->hkRoot, pRegistration->sczProviderKey);
+        if (SUCCEEDED(hr))
+        {
+            LogId(REPORT_VERBOSE, MSG_DEPENDENCY_BUNDLE_UNREGISTERED, pRegistration->sczProviderKey);
+        }
+        else if (FAILED(hr) && E_FILENOTFOUND != hr)
+        {
+            LogId(REPORT_VERBOSE, MSG_DEPENDENCY_BUNDLE_UNREGISTERED_FAILED, pRegistration->sczProviderKey, hr);
+        }
     }
 
     // Best effort to make sure this bundle is not registered as a dependent for anything.
@@ -757,6 +764,13 @@ extern "C" void DependencyUnregisterBundle(
         const BURN_PACKAGE* pPackage = &pRegistration->relatedBundles.rgRelatedBundles[i].package;
         UnregisterPackageDependency(pPackage->fPerMachine, pPackage, wzDependentProviderKey);
     }
+
+    // Best effort to make sure package providers are removed if unused.
+    for (DWORD i = 0; i < pPackages->cPackages; ++i)
+    {
+        const BURN_PACKAGE* pPackage = pPackages->rgPackages + i;
+        UnregisterOrphanPackageProviders(pPackage);
+    }
 }
 
 extern "C" HRESULT DependencyDetectCompatibleEntry(
@@ -1431,3 +1445,34 @@ static void UnregisterPackageDependency(
         }
     }
 }
+
+static void UnregisterOrphanPackageProviders(
+    __in const BURN_PACKAGE* pPackage
+    )
+{
+    HRESULT hr = S_OK;
+    DEPENDENCY* rgDependents = NULL;
+    UINT cDependents = 0;
+    HKEY hkRoot = pPackage->fPerMachine ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+
+    for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
+    {
+        const BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i];
+
+        // Skip providers not owned by the bundle.
+        if (pProvider->fImported)
+        {
+            continue;
+        }
+
+        hr = DepCheckDependents(hkRoot, pProvider->sczKey, 0, NULL, &rgDependents, &cDependents);
+        if (SUCCEEDED(hr) && !cDependents)
+        {
+            UnregisterPackageProvider(pProvider, pPackage->sczId, hkRoot);
+        }
+
+        ReleaseDependencyArray(rgDependents, cDependents);
+        rgDependents = NULL;
+        cDependents = 0;
+    }
+}
diff --git a/src/burn/engine/registration.cpp b/src/burn/engine/registration.cpp
index 59947867..a1b1b607 100644
--- a/src/burn/engine/registration.cpp
+++ b/src/burn/engine/registration.cpp
@@ -900,12 +900,8 @@ extern "C" HRESULT RegistrationSessionEnd(
     {
         AssertSz(BOOTSTRAPPER_REGISTRATION_TYPE_NONE == registrationType, "Registration type must be NONE if resume mode is NONE");
 
-        // If we own the bundle dependency then remove it.
-        if (!pRegistration->fDetectedForeignProviderKeyBundleId)
-        {
-            // Remove the bundle dependency key.
-            DependencyUnregisterBundle(pRegistration, pPackages);
-        }
+        // Remove the bundle dependencies.
+        DependencyUnregisterBundle(pRegistration, pPackages);
 
         // Delete update registration key.
         if (pRegistration->update.fRegisterUpdate)
diff --git a/src/ext/Dependency/ca/wixdepca.cpp b/src/ext/Dependency/ca/wixdepca.cpp
index 88342217..e9278e04 100644
--- a/src/ext/Dependency/ca/wixdepca.cpp
+++ b/src/ext/Dependency/ca/wixdepca.cpp
@@ -338,6 +338,10 @@ static HRESULT EnsureAbsentDependents(
 
         // Check the registry to see if the provider has any dependents registered.
         hr = DepCheckDependents(hkHive, sczProviderKey, iAttributes, sdIgnoredDependents, &rgDependents, &cDependents);
+        if (E_FILENOTFOUND == hr)
+        {
+            hr = S_OK;
+        }
         ExitOnFailure(hr, "Failed dependents check for %ls.", sczId);
     }
 
diff --git a/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
index e0b402ff..ecd12fb3 100644
--- a/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
+++ b/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
@@ -857,7 +857,7 @@ namespace WixToolsetTest.BurnE2E
             bundleA.VerifyExeTestRegistryRootDeleted(testRegistryValueExe);
         }
 
-        [Fact(Skip = "https://github.com/wixtoolset/issues/issues/3850")]
+        [Fact]
         public void RemovesDependencyProviderFromUpgradedPackageDuringUninstall()
         {
             var packageC = this.CreatePackageInstaller("PackageC");
-- 
cgit v1.2.3-55-g6feb