Use a weak event handler pattern for GlobalOptionService

src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests_EditorFeatures.cs

@@ -1453,7 +1453,7 @@ public void TestOptionChangedHandlerInvokedAfterCurrentSolutionChanged()
             Assert.Equal(FormattingOptions2.IndentStyle.Smart, secondaryWorkspace.Options.GetOption(optionKey));
 
             // Hook up the option changed event handler.
-            primaryWorkspace.GlobalOptions.OptionChanged += OptionService_OptionChanged;
+            primaryWorkspace.GlobalOptions.AddOptionChangedHandler(this, OptionService_OptionChanged);
 
             // Change workspace options through primary workspace
             primaryWorkspace.Options = primaryWorkspace.Options.WithChangedOption(optionKey, FormattingOptions2.IndentStyle.Block);
@@ -1465,7 +1465,7 @@ public void TestOptionChangedHandlerInvokedAfterCurrentSolutionChanged()
             Assert.Equal(FormattingOptions2.IndentStyle.Block, primaryWorkspace.Options.GetOption(optionKey));
             Assert.Equal(FormattingOptions2.IndentStyle.Block, secondaryWorkspace.Options.GetOption(optionKey));
 
-            primaryWorkspace.GlobalOptions.OptionChanged -= OptionService_OptionChanged;
+            primaryWorkspace.GlobalOptions.RemoveOptionChangedHandler(this, OptionService_OptionChanged);
             return;
 
             void OptionService_OptionChanged(object sender, OptionChangedEventArgs e)

src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.OptionChangedEventSource.cs

@@ -22,12 +22,12 @@ public GlobalOptionChangedEventSource(IGlobalOptionService globalOptions, IOptio
 
             public override void Connect()
             {
-                _globalOptions.OptionChanged += OnGlobalOptionChanged;
+                _globalOptions.AddOptionChangedHandler(this, OnGlobalOptionChanged);
             }
 
             public override void Disconnect()
             {
-                _globalOptions.OptionChanged -= OnGlobalOptionChanged;
+                _globalOptions.RemoveOptionChangedHandler(this, OnGlobalOptionChanged);
             }
 
             private void OnGlobalOptionChanged(object? sender, OptionChangedEventArgs e)

src/EditorFeatures/Test/Options/GlobalOptionsTests.cs

@@ -77,10 +77,6 @@ public T GetOption<T>(OptionKey2 optionKey)
 
         #region Unused
 
-#pragma warning disable CS0067
-        public event EventHandler<OptionChangedEventArgs>? OptionChanged;
-#pragma warning restore
-
         public ImmutableArray<object?> GetOptions(ImmutableArray<OptionKey2> optionKeys)
             => throw new NotImplementedException();
 
@@ -99,6 +95,12 @@ public void SetGlobalOption(OptionKey2 optionKey, object? value)
         public bool SetGlobalOptions(ImmutableArray<KeyValuePair<OptionKey2, object?>> options)
             => throw new NotImplementedException();
 
+        public void AddOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler)
+            => throw new NotImplementedException();
+
+        public void RemoveOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler)
+            => throw new NotImplementedException();
+
         #endregion
     }
 

src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs

@@ -1827,12 +1827,12 @@ public Analyzer(IGlobalOptionService globalOptions, bool waitForCancellation = f
                 this.BlockEvent = new ManualResetEventSlim(initialState: false);
                 this.RunningEvent = new ManualResetEventSlim(initialState: false);
 
-                _globalOptions.OptionChanged += GlobalOptionChanged;
+                _globalOptions.AddOptionChangedHandler(this, GlobalOptionChanged);
             }
 
             public void Shutdown()
             {
-                _globalOptions.OptionChanged -= GlobalOptionChanged;
+                _globalOptions.RemoveOptionChangedHandler(this, GlobalOptionChanged);
             }
 
             private void GlobalOptionChanged(object sender, OptionChangedEventArgs e)

src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs

@@ -65,7 +65,7 @@ public DiagnosticIncrementalAnalyzer(
 
             _diagnosticAnalyzerRunner = new InProcOrRemoteHostAnalyzerRunner(analyzerInfoCache, analyzerService.Listener);
 
-            GlobalOptions.OptionChanged += OnGlobalOptionChanged;
+            GlobalOptions.AddOptionChangedHandler(this, OnGlobalOptionChanged);
         }
 
         private void OnGlobalOptionChanged(object? sender, OptionChangedEventArgs e)
@@ -110,7 +110,7 @@ private void OnProjectAnalyzerReferenceChanged(object? sender, ProjectAnalyzerRe
 
         public void Shutdown()
         {
-            GlobalOptions.OptionChanged -= OnGlobalOptionChanged;
+            GlobalOptions.RemoveOptionChangedHandler(this, OnGlobalOptionChanged);
 
             var stateSets = _stateManager.GetAllStateSets();
 

src/Tools/ExternalAccess/Razor/RazorGlobalOptions.cs

@@ -51,10 +51,6 @@ public static RazorGlobalOptions GetGlobalOptions(Workspace workspace)
 
         private sealed class TestGlobalOptionService : IGlobalOptionService
         {
-#pragma warning disable CS0067 // Remove unused event
-            public event EventHandler<OptionChangedEventArgs>? OptionChanged;
-#pragma warning restore
-
             public T GetOption<T>(PerLanguageOption2<T> option, string languageName)
                 => default!;
 
@@ -67,6 +63,8 @@ public T GetOption<T>(PerLanguageOption2<T> option, string languageName)
             public void SetGlobalOption<T>(PerLanguageOption2<T> option, string language, T value) => throw new NotImplementedException();
             public void SetGlobalOption(OptionKey2 optionKey, object? value) => throw new NotImplementedException();
             public bool SetGlobalOptions(ImmutableArray<KeyValuePair<OptionKey2, object?>> options) => throw new NotImplementedException();
+            public void AddOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler) => throw new NotImplementedException();
+            public void RemoveOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler) => throw new NotImplementedException();
 
             bool IOptionsReader.TryGetOption<T>(OptionKey2 optionKey, out T value)
             {

src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs

@@ -79,7 +79,7 @@ public InheritanceMarginViewMargin(
             _tagAggregator.BatchedTagsChanged += OnTagsChanged;
             _textView.LayoutChanged += OnLayoutChanged;
             _textView.ZoomLevelChanged += OnZoomLevelChanged;
-            _globalOptions.OptionChanged += OnGlobalOptionChanged;
+            _globalOptions.AddOptionChangedHandler(this, OnGlobalOptionChanged);
 
             UpdateMarginVisibility();
         }
@@ -93,7 +93,7 @@ void IDisposable.Dispose()
                 _tagAggregator.BatchedTagsChanged -= OnTagsChanged;
                 _textView.LayoutChanged -= OnLayoutChanged;
                 _textView.ZoomLevelChanged -= OnZoomLevelChanged;
-                _globalOptions.OptionChanged -= OnGlobalOptionChanged;
+                _globalOptions.RemoveOptionChangedHandler(this, OnGlobalOptionChanged);
                 _tagAggregator.Dispose();
                 ((IDisposable)_glyphManager).Dispose();
             }

src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs

@@ -54,7 +54,7 @@ public VsCodeWindowManager(TLanguageService languageService, IVsCodeWindow codeW
                 _globalOptions = languageService.Package.ComponentModel.GetService<IGlobalOptionService>();
 
                 _sink = ComEventSink.Advise<IVsCodeWindowEvents>(codeWindow, this);
-                _globalOptions.OptionChanged += GlobalOptionChanged;
+                _globalOptions.AddOptionChangedHandler(this, GlobalOptionChanged);
             }
 
             private void SetupView(IVsTextView view)
@@ -227,7 +227,7 @@ public int OnNewView(IVsTextView view)
             public int RemoveAdornments()
             {
                 _sink.Unadvise();
-                _globalOptions.OptionChanged -= GlobalOptionChanged;
+                _globalOptions.RemoveOptionChangedHandler(this, GlobalOptionChanged);
 
                 if (_codeWindow is IVsDropdownBarManager dropdownManager)
                 {

src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs

@@ -30,7 +30,7 @@ private StackTraceExplorerCommandHandler(RoslynPackage package)
             _threadingContext = package.ComponentModel.GetService<IThreadingContext>();
             _globalOptions = package.ComponentModel.GetService<IGlobalOptionService>();
 
-            _globalOptions.OptionChanged += GlobalOptionChanged;
+            _globalOptions.AddOptionChangedHandler(this, GlobalOptionChanged);
 
             var enabled = _globalOptions.GetOption(StackTraceExplorerOptionsStorage.OpenOnFocus);
             if (enabled)
@@ -76,7 +76,7 @@ public void Dispose()
         {
             UnadviseBroadcastMessages();
 
-            _globalOptions.OptionChanged -= GlobalOptionChanged;
+            _globalOptions.RemoveOptionChangedHandler(this, GlobalOptionChanged);
         }
 
         private void AdviseBroadcastMessages()

src/VisualStudio/Core/Def/SymbolSearch/AbstractDelayStartedService.cs

@@ -68,7 +68,7 @@ protected AbstractDelayStartedService(
                 ProcessOptionChangesAsync,
                 listenerProvider.GetListener(FeatureAttribute.Workspace),
                 this.DisposalToken);
-            _globalOptions.OptionChanged += OnOptionChanged;
+            _globalOptions.AddOptionChangedHandler(this, OnOptionChanged);
         }
 
         protected abstract Task EnableServiceAsync(CancellationToken cancellationToken);
@@ -100,7 +100,7 @@ private async ValueTask ProcessOptionChangesAsync(CancellationToken arg)
             // We were enabled for some language.  Kick off the work for this service now. Since we're now enabled, we
             // no longer need to listen for option changes.
             _enabled = true;
-            _globalOptions.OptionChanged -= OnOptionChanged;
+            _globalOptions.RemoveOptionChangedHandler(this, OnOptionChanged);
 
             // Don't both kicking off delay-started services prior to the actual workspace being fully loaded.  We don't
             // want them using CPU/memory in the BG while we're loading things for the user.

src/VisualStudio/Core/Def/Telemetry/FileLogger.cs

@@ -39,7 +39,7 @@ public FileLogger(IGlobalOptionService globalOptions, string logFilePath)
             _buffer = new();
             _taskQueue = new(AsynchronousOperationListenerProvider.NullListener, TaskScheduler.Default);
             _enabled = globalOptions.GetOption(VisualStudioLoggingOptionsStorage.EnableFileLoggingForDiagnostics);
-            globalOptions.OptionChanged += OptionService_OptionChanged;
+            globalOptions.AddOptionChangedHandler(this, OptionService_OptionChanged);
         }
 
         public FileLogger(IGlobalOptionService optionService)

src/VisualStudio/IntegrationTest/New.IntegrationTests/Options/GlobalOptionsTest.cs

@@ -91,7 +91,7 @@ storage is VisualStudioOptionStorage.RoamingProfileStorage &&
                 await vsSettingsPersister.PersistAsync(storage, key, differentValue);
 
                 // make sure we fetch the value from the storage:
-                globalOptions.ClearCachedValues();
+                globalOptions.GetTestAccessor().ClearCachedValues();
 
                 object? updatedValue;
                 try

src/VisualStudio/VisualStudioDiagnosticsToolWindow/OptionPages/ForceLowMemoryMode.cs

@@ -22,7 +22,7 @@ public ForceLowMemoryMode(IGlobalOptionService globalOptions)
         {
             _globalOptions = globalOptions;
 
-            globalOptions.OptionChanged += Options_OptionChanged;
+            globalOptions.AddOptionChangedHandler(this, Options_OptionChanged);
 
             RefreshFromSettings();
         }

src/Workspaces/Core/Portable/Options/GlobalOptionService.cs

@@ -31,7 +31,7 @@ internal sealed class GlobalOptionService : IGlobalOptionService
 
         #endregion
 
-        public event EventHandler<OptionChangedEventArgs>? OptionChanged;
+        private readonly WeakEvent<OptionChangedEventArgs> _optionChanged = new();
 
         [ImportingConstructor]
         [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
@@ -236,27 +236,46 @@ public bool RefreshOption(OptionKey2 optionKey, object? newValue)
             return true;
         }
 
+        public void AddOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler)
+        {
+            _optionChanged.AddHandler(target, handler);
+        }
+
+        public void RemoveOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler)
+        {
+            _optionChanged.RemoveHandler(target, handler);
+        }
+
         private void RaiseOptionChangedEvent(List<OptionChangedEventArgs> changedOptions)
         {
             Debug.Assert(changedOptions.Count > 0);
 
-            // Raise option changed events.
-            var optionChanged = OptionChanged;
-            if (optionChanged != null)
+            foreach (var changedOption in changedOptions)
             {
-                foreach (var changedOption in changedOptions)
-                {
-                    optionChanged(this, changedOption);
-                }
+                _optionChanged.RaiseEvent(this, changedOption);
             }
         }
 
-        // for testing
-        public void ClearCachedValues()
+        internal TestAccessor GetTestAccessor()
         {
-            lock (_gate)
+            return new TestAccessor(this);
+        }
+
+        internal readonly struct TestAccessor
+        {
+            private readonly GlobalOptionService _instance;
+
+            internal TestAccessor(GlobalOptionService instance)
             {
-                _currentValues = ImmutableDictionary.Create<OptionKey2, object?>();
+                _instance = instance;
+            }
+
+            public void ClearCachedValues()
+            {
+                lock (_instance._gate)
+                {
+                    _instance._currentValues = ImmutableDictionary.Create<OptionKey2, object?>();
+                }
             }
         }
     }

src/Workspaces/Core/Portable/Options/IGlobalOptionService.cs

@@ -43,21 +43,19 @@ internal interface IGlobalOptionService : IOptionsReader
         /// Sets and persists the value of a global option.
         /// Sets the value of a global option.
         /// Invokes registered option persisters.
-        /// Triggers <see cref="OptionChanged"/>.
+        /// Triggers option changed event for handlers registered with <see cref="AddOptionChangedHandler"/>.
         /// </summary>
         void SetGlobalOption(OptionKey2 optionKey, object? value);
 
         /// <summary>
         /// Atomically sets the values of specified global options. The option values are persisted.
-        /// Triggers <see cref="OptionChanged"/>.
+        /// Triggers option changed event for handlers registered with <see cref="AddOptionChangedHandler"/>.
         /// </summary>
         /// <remarks>
         /// Returns true if any option changed its value stored in the global options.
         /// </remarks>
         bool SetGlobalOptions(ImmutableArray<KeyValuePair<OptionKey2, object?>> options);
 
-        event EventHandler<OptionChangedEventArgs>? OptionChanged;
-
         /// <summary>
         /// Refreshes the stored value of an option. This should only be called from persisters.
         /// Does not persist the new option value.
@@ -66,5 +64,9 @@ internal interface IGlobalOptionService : IOptionsReader
         /// Returns true if the option changed its value stored in the global options.
         /// </remarks>
         bool RefreshOption(OptionKey2 optionKey, object? newValue);
+
+        void AddOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler);
+
+        void RemoveOptionChangedHandler(object target, EventHandler<OptionChangedEventArgs> handler);
     }
 }

src/Workspaces/CoreTest/WorkspaceServiceTests/GlobalOptionServiceTests.cs

@@ -175,7 +175,7 @@ public void GlobalOptions()
             var changedOptions = new List<OptionChangedEventArgs>();
 
             var handler = new EventHandler<OptionChangedEventArgs>((_, e) => changedOptions.Add(e));
-            globalOptions.OptionChanged += handler;
+            globalOptions.AddOptionChangedHandler(this, handler);
 
             var values = globalOptions.GetOptions(ImmutableArray.Create(new OptionKey2(option1), new OptionKey2(option2)));
             Assert.Equal(1, values[0]);
@@ -201,7 +201,7 @@ public void GlobalOptions()
             Assert.Equal(6, globalOptions.GetOption(option2));
             Assert.Equal(3, globalOptions.GetOption(option3));
 
-            globalOptions.OptionChanged -= handler;
+            globalOptions.RemoveOptionChangedHandler(this, handler);
         }
 
         [Fact]

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems

@@ -538,6 +538,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Utilities\SpecializedTasks.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Utilities\StringBreaker.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Utilities\ValueTaskExtensions.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Utilities\WeakEvent`1.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Utilities\WordSimilarityChecker.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Utilities\StringEscapeEncoder.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Utilities\SyntaxPath.cs" />