Wednesday, March 16, 2005

Singleton Question

I've been going back and forth trying to decide whether or not to use a Singleton pattern for a specific situation and I've decided I need community input. In my application I have a global "object repository" which is basically just a single point of entry for application-wide services. Some objects are expensive to create and only used in certian situations but what they all have in common is that there only needs to be one instance of each service for my entire application since they are all thread safe (I hope! :P). The object repository itself contains only shared (static) readonly properties used to access the application services and has no instance methods itself. The services need to have an inheritable class hierarchy to support different application's needs.

My question is then, since I only access these objects from the ObjectRepository, do I gain anything by creating the base services as extendable (inheritable) singletons or should I just access them as shared members directly? Now what gets a little trickier is that the application is a server-side application and objects are activated as single-call objects, so depending on the load on the server the application service objects may not be around and may need to be created on the next round if the aspnet worker process is recycled, right? Anyways, I've got the system scaling really well not using the singleton pattern and just accessing the services as shared members of the ObjectRepository, but I'm just curious if using the singleton pattern in this situation will be more performant. Of course, since I need to have the services inheritable, if I go with the singletn pattern I will have to maintain the pattern on all the sub-classes so code maintenance has a weigh-in factor here as well.

So let me show you what I mean. Should I do this (what I'm doing now):
Public Class MyBaseService1

 Public Sub New()
  Initialize()
 End Sub

 Private Sub Initialize()
  'do initialization stuff
 End Sub

 Public Sub TemplateMethod()
  HookMethod1()
  HookMethod2()
 End Sub

 Protected Overridable Sub HookMethod1()
  'do default stuff
 End Sub

 Protected Overridable Sub HookMethod2()
  'do more default stuff
 End Sub
End Class


Public Class MyExtendedService1
 Inherits MyBaseService1

 Public Sub New()
  MyBase.New()
 End Sub

 Protected Overrides Sub HookMethod1()
  'do different stuff
 End Sub

 Protected Overrides Sub HookMethod2()
  'do more different stuff
 End Sub
End Class


Public Class ObjectRepository1

 Private Shared m_service1 As MyExtendedService1
 Private Shared _lock As New Object

 'Use lazy initialization because this 
 ' class is rarely used and expensive to create.
 Public Shared ReadOnly Property Service1() As MyExtendedService1
  Get
   If m_service1 Is Nothing Then
    SyncLock (_lock)
     If m_service1 Is Nothing Then
      m_service1 = New MyExtendedService1
     End If
    End SyncLock
   End If
   Return m_service1
  End Get
 End Property

End Clas
Or would it be better to implement this pattern?
Public Class MyBaseService
 Protected Shared _Instance As MyBaseService
 Protected Shared _Lock As New Object

 Protected Sub New()
  Initialize()
 End Sub

 Public Shared Function GetInstance() As MyBaseService
  If _Instance Is Nothing Then
   SyncLock (_Lock)
    If _Instance Is Nothing Then
     _Instance = New MyBaseService
    End If
   End SyncLock
  End If
  Return _Instance
 End Function

 Private Sub Initialize()
  'do initialization stuff
 End Sub

 Public Sub TemplateMethod()
  HookMethod1()
  HookMethod2()
 End Sub

 Protected Overridable Sub HookMethod1()
  'do default stuff
 End Sub

 Protected Overridable Sub HookMethod2()
  'do more default stuff
 End Sub
End Class


Public Class MyExtendedService
 Inherits MyBaseService

 Public Shared Shadows Function GetInstance() As MyExtendedService
  If _Instance Is Nothing Then
   SyncLock (_Lock)
    If _Instance Is Nothing Then
     _Instance = New MyExtendedService
    End If
   End SyncLock
  End If
  Return DirectCast(_Instance, MyExtendedService)
 End Function

 Protected Overrides Sub HookMethod1()
  'do different stuff
 End Sub

 Protected Overrides Sub HookMethod2()
  'do more different stuff
 End Sub
End Class

Public Class ObjectRepository
 Private Shared m_service1 As MyExtendedService

 'Use lazy initialization because this 
 ' class is rarely used and expensive to create.
 Public Shared ReadOnly Property Service1() As MyExtendedService
  Get
   If m_service1 Is Nothing Then
    m_service1 = MyExtendedService.GetInstance()
   End If
   Return m_service1
  End Get
 End Property

End Class
I mean, I really don't see the advantage of this pattern for my situation but maybe I'm missing something? Thanks in advance!

7 comments:

Jake Good said...

IMHO, if it's scaling well... and your data is not getting mangled by threads or concurrent operations... then why change it up?

You could potentially test out different scenerios for scaling and concurrency within unit tests, providing you with even more proof of whether you should switch or not...

I typically use singleton patterns for data or classes that you want to restrict to single instances, not classes that may work better with single instances. :: shrugs ::

S dot One said...

The next article might be interesting to you :
Implementing the Singleton Pattern in C#

I based one of my singleton implementation on it, you can find the vb.net code over here (please do not try to read the Dutch part) : http://www.heefthetover.net/sdotone/?p=14

Beth Massi said...

Thanks for the tips. The article you point to is quite interesting and has raised even more questions for me :-). I was under the impression that the double-checked locking pattern did work in .NET. (See this MSDN article) If not, what is the proper way to implement a thread-safe lazy initialization pattern in .NET? Can someone point me to more information on proof that the double checked locking pattern is broken in .NET?

Anonymous said...

Check out the post I made why singletons are evil. Just more to think about. http://blogs.msdn.com/scottdensmore/archive/2004/05/25/140827.aspx

Beth Massi said...

I think I'm going to leave my code as is because the ObjectRepository maintains the single shared instances of the application services and those service classes can be inherited easier if they do not implement the singleton pattern (Scott, thanks for the convincing). However, since my ObjectRepository only calls the service factory to create the shared service when it is requested (lazy initialization), I am concerned how to implement this pattern in a thread-safe way. Right now I am using the double-checked locking pattern. Is this is not a good idea? If not, what's the solution?

Anonymous said...

Oh cuncurrency how I hate thee. Your best bet is the double checked locking, but know that you can still get a race condition so your variable needs to be volatile. Read this (http://blogs.msdn.com/brada/archive/2004/05/12/130935.aspx) to get the answer of why. I don't know VB.NET (I am a C++ / C# guy), but I am sure they have the same idea of a volatile in VB.NET.

scott

Beth Massi said...

Oh my god, Magnum! Ok so it sounds like I should just do the Lock every time the read is attempted to be on the safe side and forget this double-check locking patten. Memory barriers are giving me a headache. But the thing is these shared members are actually in memory until the aspnet worker process is recycled so there is only a very very very slim chance that I'll ever see a race condition in this situation. But better safe than sorry, I admit. I'm going to do some performace tests and I'll post my final descision. Thanks for all the help.