4

I implemented an HTTP server in c# .NET:

public class HttpServer
{
    private HttpListener listener;

    public HttpServer()
    {
        listener = new HttpListener();
        listener.Prefixes.Add("http://localhost:8080/");
    }

    public void Start()
    {
        lock(this) {
            listener.Start();
            AsyncProcessing(listener);
        }
    }

    public void Stop()
    {
        lock (this) {
            listener.Stop();
        }
    }

    private void AsyncProcessing(HttpListener listener)
    {
        if (listener == null)
            return;

        listener.BeginGetContext(new AsyncCallback(Callback), listener);
    }

    private void Callback(IAsyncResult result)
    {
        HttpListenerContext context = null;

        lock(this) {
            HttpListener listener = (HttpListener)result.AsyncState;
            if (!listener.IsListening)
                return;

            context = listener.EndGetContext(result);

            AsyncProcessing(listener);
        }

        /* handle request */
    }
}

I have some questions about this implementation:

  • I added some locks here and there to prevent race conditions, but I'm confused: Are those really needed? I read in the documentation that all public non-static methods are NOT thread safe. Why haven't I seen code where this fact is considered?
  • How does the HttpListenerContext behave? Does he have some sort of connection to the HttpListener? Or can I use multiple HttpListenerContexts concurrently?
  • I heard HttpListener wouldn't be ready for production systems, but I've never seen an argument supporting this claim. Is it true or not?
  • Are there other things I should consider which I haven't mentioned?

Thanks for your ideas

5
  • 2
    There is no built-in thread-safety of static or non-static methods, you have to provide all of that yourself. Typically, code is not written to be completely thread-safe since that usually entails more than just a few locks here and there. In other words, whether the class and/or method is static or not has no bearing on the thread-safety of the code. Jun 16, 2011 at 12:26
  • the msdn explicitly says that the public static members are thread safe. but that's not important, since I don't use any static members..
    – duedl0r
    Jun 16, 2011 at 12:31
  • Do you have citations or a quote for the "not production ready" part of your question? I can't seem to ever having seen such a comment and would be interested in reading the reasoning behind it. Note, this is me not saying that that quote is wrong, I'm saying I haven't heard it, and would like to know more. Jun 16, 2011 at 12:41
  • Ok, my first comment was about static and non-static methods in general, a class might document that all static methods are thread-safe though, but as my comment says, they built that into the methods, making them static has nothing to do with it. It was a design and implementation choice by Microsoft. Jun 16, 2011 at 12:42
  • @thread-safety: ok, good to know. @httplistener: yes, I also want to know more about that. Unfortunately I don't remember where I read those comments.
    – duedl0r
    Jun 16, 2011 at 14:03

1 Answer 1

4

Bear in mind that I'm no expert in multithreading, so you should take care to verify, as best as you can, anything I say.

If anyone else knows, and would like to just steal my entire answer and just edit in or correct the details, feel free to do that.

Let's deal with your questions one by one:

I added some locks here and there to prevent race conditions, but I'm confused: Are those really needed? I read in the documentation that all public non-static methods are NOT thread safe. Why haven't I seen code where this fact is considered?

Well, yes and no. Locks are typically used to prevent multiple threads to access the same data structure at the same time, since it would corrupt the data structure. Consider sorting an array on one thread and inserting an element in the middle in another, timing those two threads correct would corrupt the contents of the array.

Now, in your code you are locking on this which is never a good idea. Outside code might also take a lock on the same object, and that's out of your control, so in the interest of creating production ready code, I would not do that.

If you need locks in your code, I would construct specific lock objects and use those.

In other words, add this:

private readonly object _Lock = new object();

and then everywhere you have lock(this) replace it with lock(_Lock) instead. This way you can also have multiple locks, if needs be.

As for actually needing locks, I'm not 100% sure on that. The thing I'm not sure about is that you're locking before calling Stop, and you're locking in the callback and inside the lock you check if the listener is still running.

This will prevent you from stopping the listener after accepting a request, but before you've actually processed the request. In other words, it sounds like you would prevent closing the server with open requests still being handled.

But, no, you wouldn't prevent that, because you might stop the server after leaving the locked section in the callback, but during or before the commented code has fully executed, so you would still have that problem.

However It will also mean you've effectively serialized some of the callback method, the part where you call EndGetContext and restart the BeginGetContext cycle. Whether this is a good pattern or not, I don't know.

How does the HttpListenerContext behave? Does he have some sort of connection to the HttpListener? Or can I use multiple HttpListenerContexts concurrently?

Here I will make a guess. That class has no reference back to the listener class, or, it has a thread-safe way of cooperating with it.

It wouldn't be much of a thread-based http listener system if every access to the request/response data has to be serialized.

In any case, if in doubt, check the documentation of the methods and/or properties on the context class that you're accessing, and if you need to take steps to ensure thread safety, the documentation will say so.

I heard HttpListener wouldn't be ready for production systems, but I've never seen an argument supporting this claim. Is it true or not?

(see comment on question)

Are there other things I should consider which I haven't mentioned?

Multi-threaded code is hard to write. Judging by your question I would venture a guess that you haven't done a lot of it, and to be honest, though I have done a lot of it, I still feel like I'm on thin ice.

My advice is thus as follows:

  • Do you really need multi-threading?
  • Do you have other colleagues that know more about this that could help you?
4
  • Upon re-reading my answer I don't feel like I actually answered much of the questions, more like an opiniated dump of thoughts. Feel free to flag as "not an answer". I'm leaving it for now if someone wants to pilfer some text for a better answer though. Jun 16, 2011 at 12:40
  • The thing is, I only understand the concepts behind multi-threaded coding. I don't have any experience doing that in c#. So I read HttpListener MSDN and realized there were no comments about this topic. This is why I got suspicious and wanted to clear things up.. Then I inserted some locks to protect the listener object, since it isn't thread safe (good tip btw @private lock object, thx). I thought it is really bad if I operate on the listener object while calling the Stop method :)
    – duedl0r
    Jun 16, 2011 at 14:17
  • It is also my guess that the context isn't depending on the listener, that's why the handling of the request isn't locked. But IMHO it's a bit scary not knowing what's really happening, MSDN doesn't talk about this at all. Furthermore, if context and listener don't depend on each other, there wouldn't be any problems stopping the listener and handling the remaining requests, right? So, your comment might be contradicting, or did I misread you?
    – duedl0r
    Jun 16, 2011 at 14:22
  • 1
    No, you're absolutely right, and I must confess that I don't really know what would happen if you stop the listener while still having requests in motion so to speak. I bet that if you asked that part as its own question, you might get much better answers than my opinions above. Also, for the future, it might be wise to split up a question by taking one thing at a time, generally that lead to better and more qualified answers here on SO. Jun 16, 2011 at 15:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.