[Banshee-devel-list] Re-entrant locks in ServiceManager

[Ruben Vermeersch <ruben savanne be>]

Goodmorning!

I was just checking out ServiceManager and I noticed the following:

        private static void RegisterServiceNoLock (IService service)
        {
            services.Add (service.ServiceName, service);
            
            if(service is IDBusExportable) {
                DBusServiceManager.RegisterObject
((IDBusExportable)service);
            }
        }
                    
        public static void RegisterService (IService service)
        {
            lock (self_mutex) {
                RegisterServiceNoLock (service);
            }
        }
        
Is there any specific reason to have two copies of this method? The lock
statement in C# is re-entrant, as such, I would like to proposed
attached patch, which gives no noticeable difference in starting up.

However, as I'm not sure why you wrote it like this, I wanted to check
up with you first. Is there any reason for the two methods
(performance?)? If so, do enlighten me, I'm here to learn.

Otherwise, do you have any objections against me committing this? Less
code always makes me feel better.

On a side note:  // TODO: Add something like
ServiceManager.NotifyStartup ("InterfaceActionService", Initialize);

Is this because you can't (at least not that I know of) have
parametrised/generic events? Basically a workaround for the lack of:

ServiceManager.ServiceStarted<InterfaceActionService> += delegate
Initialize;


Cheerios!
   Ruben


PS: Yes, I should've been studying...
PS2: Putting -devel on CC, in case anyone happens to know about some
smart trick with events.


--
Ruben Vermeersch (rubenv)
http://www.savanne.be

Index: src/Core/Banshee.Services/Banshee.ServiceStack/ServiceManager.cs
===================================================================
--- src/Core/Banshee.Services/Banshee.ServiceStack/ServiceManager.cs	(revision 3029)
+++ src/Core/Banshee.Services/Banshee.ServiceStack/ServiceManager.cs	(working copy)
@@ -85,13 +85,13 @@
                 
                 foreach (Type type in service_types) {
                     IService service = (IService)Activator.CreateInstance (type);
-                    RegisterServiceNoLock (service);
+                    RegisterService (service);
                     OnServiceStarted (service);
                 }
                 
                 foreach (TypeExtensionNode node in extension_nodes) {
                     IService service = (IService)node.CreateInstance (typeof (IService));
-                    RegisterServiceNoLock (service);
+                    RegisterService (service);
                     OnServiceStarted (service);
                 }
                 
@@ -114,19 +114,14 @@
             }
         }
         
-        private static void RegisterServiceNoLock (IService service)
-        {
-            services.Add (service.ServiceName, service);
-            
-            if(service is IDBusExportable) {
-                DBusServiceManager.RegisterObject ((IDBusExportable)service);
-            }
-        }
-                    
         public static void RegisterService (IService service)
         {
             lock (self_mutex) {
-                RegisterServiceNoLock (service);
+                services.Add (service.ServiceName, service);
+                
+                if(service is IDBusExportable) {
+                    DBusServiceManager.RegisterObject ((IDBusExportable)service);
+                }
             }
         }
         
@@ -134,7 +129,7 @@
         {
             lock (self_mutex) {
                 if (is_initialized) {
-                    RegisterServiceNoLock (Activator.CreateInstance <T> ());
+                    RegisterService (Activator.CreateInstance <T> ());
                 } else {
                     service_types.Add (typeof (T));
                 }
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 3029)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2008-01-26  Ruben Vermeersch  <ruben savanne be>
+
+	* src/Core/Banshee.Services/Banshee.ServiceStack/ServiceManager.cs: C#'s
+	lock statement is re-entrant. Therefore, there's no need to have both
+	RegisterService and RegisterServiceNoLock.
+
 2008-01-25  Aaron Bockover  <abock gnome org>
 
 	* src/Core/Banshee.Services/Banshee.Library/LibraryImportManager.cs: