From 0d7fc9ba0cdfd59dcd88f36d16cb6fef03c6341b Mon Sep 17 00:00:00 2001 From: Chow Loong Jin Date: Mon, 7 Nov 2011 10:19:35 +0800 Subject: [PATCH 1/2] Protect BrowseService against recursive Dispose() Removing event listeners on the resolver can result in recursive calls to BrowseService.Dispose(), caused by signals being dispatched inside `resolver.Failure -= OnResolveFailure', rendering the atomicity guaranteed by the lock (this) block ineffective. This commit fortifies BrowseService and ServiceBrowser from this: - BrowseService.Dispose() will not call DisposeResolver() when already disposed. - BrowseService.DisposeResolver() will nullify this.resolver before performing cleanup on the resolver. - ServiceBrowser.OnItemNew() will replace the service entry in its services dictionary with the new service before disposing of the old one. - ServiceBrowser.OnItemRemoved() will remove the service entry from its services dictionary before running Dispose() on it, thus preventing recursive Dispose() calls. --- .../BrowseService.cs | 10 +++++++--- .../ServiceBrowser.cs | 15 +++++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/BrowseService.cs b/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/BrowseService.cs index 01a0aea..46d3e55 100644 --- a/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/BrowseService.cs +++ b/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/BrowseService.cs @@ -52,19 +52,23 @@ public BrowseService (string name, string regtype, string replyDomain, int @inte public void Dispose () { lock (this) { - disposed = true; - DisposeResolver (); + if (!disposed) { + disposed = true; + DisposeResolver (); + } } } private void DisposeResolver () { lock (this) { + IAvahiServiceResolver resolver = this.resolver; + if (resolver != null) { + this.resolver = null; resolver.Failure -= OnResolveFailure; resolver.Found -= OnResolveFound; resolver.Free (); - resolver = null; } } } diff --git a/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs b/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs index 9efe73e..1500500 100644 --- a/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs +++ b/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs @@ -117,10 +117,11 @@ private void OnItemNew (int @interface, Protocol protocol, string name, string t { lock (this) { BrowseService service = new BrowseService (name, type, domain, @interface, protocol); - - if (services.ContainsKey (name)) { - services[name].Dispose (); + BrowseService to_dispose; + + if (services.TryGetValue (name, out to_dispose)) { services[name] = service; + to_dispose.Dispose (); } else { services.Add (name, service); } @@ -134,10 +135,12 @@ private void OnItemRemove (int @interface, Protocol protocol, string name, strin { lock (this) { BrowseService service = new BrowseService (name, type, domain, @interface, protocol); - - if (services.ContainsKey (name)) { - services[name].Dispose (); + BrowseService to_dispose; + + // handler may be called recursively, so remove service from services before disposing + if (services.TryGetValue (name, out to_dispose)) { services.Remove (name); + to_dispose.Dispose (); } OnServiceRemoved (service); From 2894e5c1560d8bbc4a10f608c04202a326527af3 Mon Sep 17 00:00:00 2001 From: Chow Loong Jin Date: Mon, 7 Nov 2011 10:51:01 +0800 Subject: [PATCH 2/2] Allow ServiceBrowser.Dispose() to be called again Dispose methods of IDisposable objects are should be able to be called more than once, so avoid doing anything after the first time. --- .../ServiceBrowser.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs b/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs index 1500500..2871525 100644 --- a/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs +++ b/src/Mono.Zeroconf.Providers.AvahiDBus/Mono.Zeroconf.Providers.AvahiDBus/ServiceBrowser.cs @@ -45,17 +45,21 @@ public class ServiceBrowser : IServiceBrowser public void Dispose () { lock (this) { + IAvahiServiceBrowser service_browser = this.service_browser; if (service_browser != null) { + this.service_browser = null; service_browser.ItemNew -= OnItemNew; service_browser.ItemRemove -= OnItemRemove; service_browser.Free (); } - + if (services.Count > 0) { - foreach (BrowseService service in services.Values) { + List services_list = new List (services.Values); + services.Clear (); // Clear first so we no-op if we Dispose() again + + foreach (BrowseService service in services_list) { service.Dispose (); } - services.Clear (); } } }