DefaultControllerFactory不是线程安全的

由于项目需要，刚才打算为ASP.NET MVC应用程序增强ControllerFactory的功能，因此翻出了ASP.NET MVC的源代码开始阅读其DefaultControllerFactory。代码不多，很容易理解，不过读着读着便发现了问题，因为我发现DefaultControllerFactory不是线程安全的。

线程安全，故名思义便是在多线程的环境下，是否可以正常工作的意思。以前也看过ASP.NET MVC的源代码，印象中在默认情况下，整个应用程序会共享同一个DefaultControllerFactory实例（刚才又确认了一下的确是这样的），这便要求这个类的所有操作——至少是对外公开的接口都需要是线程安全的。

线程安全与否有时候并不容易发现，但是某些状况下我们还是可以总结出一些“实现模式”，遵循或违反这些模式，则这个组件很可能不是线程安全的。例如，如果满足了Share Nothing，至少是No Shared Writing，这个组件很可能就是线程安全的。不过DefaultControllerFactory可不是这样，例如它的CreateController方法：

public virtual IController CreateController(RequestContext requestContext, string controllerName) {
    if (requestContext == null) {
        throw new ArgumentNullException("requestContext");
    }
    if (String.IsNullOrEmpty(controllerName)) {
        throw new ArgumentException(MvcResources.Common_NullOrEmpty, "controllerName");
    }
    RequestContext = requestContext;
    Type controllerType = GetControllerType(controllerName);
    IController controller = GetControllerInstance(controllerType);
    return controller;
}

请看这行红色代码，这是一句为自身实例属性RequestContent赋值的语句，它自然是要被接下来的GetControllerType和GetControllerInstance方法所访问。当我们发现了类似的“写属性”的语句应该有所警觉，因为它往往意味着这个组件不是线程安全的。如果是线程安全的代码，则往往需要将数据统统作为方法的参数进行传递。

那么，既然DefaultControllerFactory不是线程安全的，那么为什么在使用过程中却没有发生任何问题呢？幸运的是，在普通使用过程中，我们几乎无法遇到这样的逻辑。例如在GetControllerType中，对RequestContext属性读取的逻辑是这样的：

protected internal virtual Type GetControllerType(string controllerName) {
    ...

    // first search in the current route's namespace collection
    object routeNamespacesObj;
    Type match;
    if (RequestContext != null && 
        RequestContext.RouteData.DataTokens.TryGetValue("Namespaces", out routeNamespacesObj)) {
        ...
    }

    ...
}

RequestContext永远不可能为null，而且我也从来没有见过谁向RouteData中的DataTokens集合中填充过数据（这需要在应用程序一开始进行Route Mapping时指定），因此这段逻辑“永远”是略过的。GetControllerInstance方法情况略有不同：

protected internal virtual IController GetControllerInstance(Type controllerType) {
    if (controllerType == null) {
        throw new HttpException(404,
            String.Format(
                CultureInfo.CurrentUICulture,
                MvcResources.DefaultControllerFactory_NoControllerFound,
                RequestContext.HttpContext.Request.Path));
    }

    ...
}

只有在ControllerType为null的时候，为了抛出一个404异常才会涉及到ReqeustContext属性——只是，这时候又有谁去注意这个呢？

就是这样，这个问题被“幸运”地绕开了。

如果您觉得这并不是什么严重的问题，自然可以放任不管。如果您觉得这个问题需要解决的话，其实也只需要在Application Start时加入这样一段话就可以了：

ControllerBuilder.Current.SetControllerFactory(typeof(DefaultControllerFactory));

于是在每次请求时，ControllerBuilder都会使用Activator.CreateInstance来创建一个新的对象，这样就不会出现多线程方面的问题了。只可惜，ControllerBuilder在默认情况下使用的是另一种方式：

public class ControllerBuilder {
    ...

    public ControllerBuilder() {
        SetControllerFactory(new DefaultControllerFactory() {
            ControllerBuilder = this
        });
    }
    ...
}

由于使用了SetControllerFactory方法的另一个重载并直接提供了一个DefaultControllerFactory实例，因此每个请求都会共享同一个对象了。

在修改了代码之后，我们每个请求都会创建一个新的DefaultControllerFactory实例，但是由于它的缓存（保存了controller名称与特定类型的对应关系）容器是static的，您也无需担心性能上的问题。

值得一提的是，这个问题已经在ASP.NET MVC 2 Preview 1中修复了，而它的做法是将requestContext对象作为参数传递到GetControllerType和GetControllerInstance方法中去，这自然就没有问题了：

public virtual IController CreateController(RequestContext requestContext, string controllerName) {
    if (requestContext == null) {
        throw new ArgumentNullException("requestContext");
    }
    if (String.IsNullOrEmpty(controllerName)) {
        throw new ArgumentException(MvcResources.Common_NullOrEmpty, "controllerName");
    }
    Type controllerType = GetControllerType(requestContext, controllerName);
    IController controller = GetControllerInstance(requestContext, controllerType);
    return controller;
}

产生这个的原因，我只能猜想微软没有把Code Review的工作进行到位了，我不相信这么明显的问题微软的大牛们会发现不了。什么？您说单元测试？这其实才是我打算和大家讨论的东西。对于多线程的应用程序，我们如何能够通过单元测试来验证一个组件是否有线程安全方面的问题呢？我也相信微软为这部分有问题的代码编写了单元测试（虽然我没有去找过），但是要知道目前所有的单元测试都是单线程运行的，而在单线程的情况下是无法让线程安全问题暴露出来的。更有意思的是，即使是线程不安全的代码，在多线程的环境下，也有可能工作正常。

那么，我们又该怎么办呢？大家一起讨论一下吧。