Windows Service Implementing IDisposable - Is it bad practice?

Question

I've come across this code:

public class ServiceLauncher2 : ServiceBase, IDisposable

And then this:

        /// <summary>
        /// Disposes the controllers
        /// </summary>
        // This is declared new as opposed to override because the base class has to be able to
        // call its own Dispose(bool) method and not this one. We could just as easily name
        // this method something different, but keeping it Dispose is just as valid. 
        public new void Dispose()
        {
            foreach (var mgr in _threadManagers)
                mgr.Dispose();
            base.Dispose();
        }

I've never seen this in a Windows Service implementation before. Usually just OnStop/OnStart is overridden. Is this bad practice?

Answer 1

Let's count the ways this is bad practice:

• The new keyword is grating, it tells the compiler to shut up about a potential problem in the code. A real one, the code that uses this class can easily end up calling ServiceBase.Dispose() instead. ServiceBase implements the disposable pattern, the correct way to do it is to override the protected Dispose(bool) method

• The Dispose() method leaves a _threadManagers collection object behind that contains nothing but dead objects. Which makes the collection dead as a doornail as well, iterating it afterwards is meaningless. It should have been emptied

• The only time this Dispose() method can be called is at service termination. Can't do it in OnStop(), it also disposed the ServiceBase. Disposing "controllers" a microsecond before the finalizers run and the process terminates makes no sense. Dispose() should only ever be used to allow unmanaged resources to be de-allocated early. There is no early when the process stops a millisecond later

This code makes no sense. Don't use it.

Answer 2

It does look non-standard but it is legit. So I wouldn't necessarily call it bad practice, though the fact that it introduces confusion makes it bad practice?

Does this run only as a service or is there console mode? (Console app would not get OnStop called.) Or is there some other (custom) way to stop this service process?

Ammending from my own earlier question of:

I'm not sure why new instead of override, especially since base.Dispose() is being called.

Reason:

'SomeClass.Dispose()': cannot override inherited member 'System.ComponentModel.Component.Dispose()' because it is not marked virtual, abstract, or override

In other words, the implementaion of ServiceBase.Dispose is not overridable.