Fix memory leak because Binding.Source (#1033)

* fix memory leak * Update dev/dll/SharedHelpers.cpp
microsoft · Jul 11, 2019 · 7d986fe · 7d986fe
1 parent b88dd7c
commit 7d986fe
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 11 deletions.
diff --git a/dev/NavigationView/NavigationView.cpp b/dev/NavigationView/NavigationView.cpp
@@ -2924,11 +2924,7 @@ void NavigationView::SwapPaneHeaderContent(tracker_ref<winrt::ContentControl> ne
             oldParent.ClearValue(winrt::ContentControl::ContentProperty());
         }
 
-        auto binding = winrt::Binding();
-        auto propertyPath = winrt::PropertyPath(propertyPathName);
-        binding.Path(propertyPath);
-        binding.Source(*this);
-        winrt::BindingOperations::SetBinding(newParent, winrt::ContentControl::ContentProperty(), binding);
+        SharedHelpers::SetBinding(propertyPathName, newParent, winrt::ContentControl::ContentProperty());
     }
 }
 
@@ -2954,11 +2950,7 @@ void NavigationView::UpdateContentBindingsForPaneDisplayMode()
             notControl.ClearValue(winrt::ContentControl::ContentProperty());
         }
 
-        auto binding = winrt::Binding();
-        auto propertyPath = winrt::PropertyPath(L"AutoSuggestBox");
-        binding.Path(propertyPath);
-        binding.Source(*this);
-        winrt::BindingOperations::SetBinding(autoSuggestBoxContentControl, winrt::ContentControl::ContentProperty(), binding);
+        SharedHelpers::SetBinding(L"AutoSuggestBox", autoSuggestBoxContentControl, winrt::ContentControl::ContentProperty());
     }
 }
 

diff --git a/dev/dll/SharedHelpers.cpp b/dev/dll/SharedHelpers.cpp
@@ -569,6 +569,28 @@ winrt::IconElement SharedHelpers::MakeIconElementFrom(winrt::IconSource const& i
     return nullptr;
 }
 
+void SharedHelpers::SetBinding(
+    std::wstring_view const& pathString,
+    winrt::DependencyObject const& target,
+    winrt::DependencyProperty const& targetProperty)
+{
+    winrt::Binding binding;
+    winrt::RelativeSource relativeSource;
+    relativeSource.Mode(winrt::RelativeSourceMode::TemplatedParent);
+    binding.RelativeSource(relativeSource);
+
+    binding.Path(winrt::PropertyPath(pathString));
+
+    winrt::BindingOperations::SetBinding(target, targetProperty, binding);
+}
+
+// Be cautious: this function may introduce memory leak because Source holds strong reference to target too
+// There’s an intermediary object – the BindingExpression when BindingOperations::SetBinding
+// For example, if source is NavigationView and target is content control,
+// and there is strong reference: NavigationView -> ContentControl
+// BindingExpression.Source also make a strong reference to NavigationView
+// and it introduces the cycle: ContentControl -> BindingExpression -> NavigationView -> ContentControl
+// Prefer to use RelativeSource version of SetBinding if possible.
 void SharedHelpers::SetBinding(
     winrt::IInspectable const& source,
     std::wstring_view const& pathString,
@@ -578,8 +600,8 @@ void SharedHelpers::SetBinding(
     winrt::BindingMode mode)
 {
     winrt::Binding binding;
-
     binding.Source(source);
+
     binding.Path(winrt::PropertyPath(pathString));
     binding.Mode(mode);
 

diff --git a/dev/inc/SharedHelpers.h b/dev/inc/SharedHelpers.h
@@ -224,6 +224,11 @@ class SharedHelpers
 
     static winrt::IconElement MakeIconElementFrom(winrt::IconSource const& iconSource);
 
+    static void SetBinding(
+        std::wstring_view const& pathString,
+        winrt::DependencyObject const& target,
+        winrt::DependencyProperty const& targetProperty);
+
     static void SetBinding(
         winrt::IInspectable const& source,
         std::wstring_view const& pathString,