Friday, May 04, 2012

How Do I Detect When A Client Thread Exits? Make The Client Tell You.

Yesterday I asked the LazyWeb for some help: how does library code detect then a client thread that has called one of its methods exits? There followed a very useful discussion on Stack Overflow. I’ve been persuaded by Martin James’ arguments that however one tries to do it, there will always be scenarios where the result won’t be optimal. The answer is for the client to tell the library explicitly when it’s finished its work.

“'it just works' solutions just don't <g>. Whidows, Linux, every OS I've ever used has a 'token', 'handle' or other such object as the solution to maintaining state across calls. Using inplicit thread-local data and polling for thread termination etc. will suck the life out of your library”

OK, so I  buy the argument, but what’s the best way to change the API so that the client can indicate that it’s finished its work?

First alternative.

Provide a method that returns a worker that implements IDisposable and make it clear in the documentation that you should not share workers between threads. Here's the modified library:

public class Library
{
public LibraryWorker GetLibraryWorker()
{
return new LibraryWorker();
}
}

public class LibraryWorker : IDisposable
{
public void StartSomething()
{
Console.WriteLine("Library says: StartSomething called");
}

public void Dispose()
{
Console.WriteLine("Library says: I can clean up");
}
}

The client is now a little more complicated:

public class Client
{
private readonly Library library;

public Client(Library library)
{
this.library = library;
}

public void DoWorkInAThread()
{
var thread = new Thread(() =>
{
using(var worker = library.GetLibraryWorker())
{
worker.StartSomething();
Console.WriteLine("Client thread says: I'm done");
}
});
thread.Start();
}
}

The main problem with this change is that it's a breaking change for the API. Existing clients will have to be re-written. Now that's not such a bad thing, it would mean revisiting them and making sure they are cleaning up correctly.

Non-breaking second alternative.

The API provides a way for the client to declare 'work scope'. Once the scope completes, the library can clean up. The library provides a WorkScope that implements IDisposable, but unlike the first alternative above, the StartSomething method stays on the Library class:

public class Library
{
public WorkScope GetWorkScope()
{
return new WorkScope();
}

public void StartSomething()
{
Console.WriteLine("Library says: StartSomething called");
}
}

public class WorkScope : IDisposable
{
public void Dispose()
{
Console.WriteLine("Library says: I can clean up");
}
}

The client simply puts the StartSomething call in a WorkScope...

public class Client
{
private readonly Library library;

public Client(Library library)
{
this.library = library;
}

public void DoWorkInAThread()
{
var thread = new Thread(() =>
{
using(library.GetWorkScope())
{
library.StartSomething();
Console.WriteLine("Client thread says: I'm done");
}
});
thread.Start();
}
}

I like this less than the first alternative because it doesn't force the library user to think about scope. What do you think?

1 comment:

Roy Jacobs said...

The first approach makes the most sense and is also closely related to how you would work with "sessions" like in ORMs such as NHibernate or RavenDB. They also have similar restrictions on thread safety.