Hello World
Spiga

DefaultControllerFactory不是线程安全的

2009-08-18 16:07 by 老赵, 6097 visits

由于项目需要,刚才打算为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的工作进行到位了,我不相信这么明显的问题微软的大牛们会发现不了。什么?您说单元测试?这其实才是我打算和大家讨论的东西。对于多线程的应用程序,我们如何能够通过单元测试来验证一个组件是否有线程安全方面的问题呢?我也相信微软为这部分有问题的代码编写了单元测试(虽然我没有去找过),但是要知道目前所有的单元测试都是单线程运行的,而在单线程的情况下是无法让线程安全问题暴露出来的。更有意思的是,即使是线程不安全的代码,在多线程的环境下,也有可能工作正常。

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

Creative Commons License

本文基于署名 2.5 中国大陆许可协议发布,欢迎转载,演绎或用于商业目的,但是必须保留本文的署名赵劼(包含链接),具体操作方式可参考此处。如您有任何疑问或者授权方面的协商,请给我留言

Add your comment

30 条回复

  1. 迷途小羔羊[未注册用户]
    *.*.*.*
    链接

    迷途小羔羊[未注册用户] 2009-08-18 16:15:00

    线程安全,故名思义便是在多线程的环境下,是否可以正常工作的意思。
    -----------------------------------------------
    查 MSDN 时经常能看见“线程安全XXXX”,
    不过我对这个很迷茫。

  2. 老赵
    admin
    链接

    老赵 2009-08-18 16:19:00

    网上逛了逛,关于单元测试检查线程安全的问题,似乎是nearly impossible的。

  3. 温景良(Jason)
    *.*.*.*
    链接

    温景良(Jason) 2009-08-18 16:30:00

    lu guo kan kan

  4. SoJin
    *.*.*.*
    链接

    SoJin 2009-08-18 16:38:00

    请问下老赵,IHttpHandle同一时间会有多个实例同时执行吗?还是说每个请求都会等上一个请求执行完才会执行?
    上次看mvc源码也“似乎”发现过这个问题,但有人告诉我同一时间只会执行一个请求,所以我就认为这个不会有影响。

    另外既然老赵看到了DefaultControllerFactory.GetControllerType
    方法能否再花点时间解答下我一周前短消息你的那个关于mvc area的疑问呢?


  5. 老赵
    admin
    链接

    老赵 2009-08-18 16:44:00

    @SoJin
    IHttpHandler每次是指执行一个,但是在ASP.NET MVC的默认实现中,它们公用一个DefaultControllerFactory实例。
    // 不好意思我忘了,我晚上就仔细看看。

  6. SoJin
    *.*.*.*
    链接

    SoJin 2009-08-18 16:56:00

    @Jeffrey Zhao
    那如果每次是执行一个的话,即使他们共用一个DefaultControllerFactory实例也应该不会出问题的吧?

    RequestContext = requestContext;
    DefaultControllerFactory的RequestContext 属性虽然每次都会变成新的,但每个Handle都会等上一个执行完毕,这样会在什么情况下出问题呢?





  7. 王丰
    *.*.*.*
    链接

    王丰 2009-08-18 16:58:00

    老赵真是高产啊,每天都更新,精力旺盛的惊人

  8. 老赵
    admin
    链接

    老赵 2009-08-18 17:03:00

    @SoJin
    我是说,每个IHttpHandler对象不会被多个线程同时执行,但是多个线程会同时执行多个IHttpHandler对象。

  9. SoJin
    *.*.*.*
    链接

    SoJin 2009-08-18 17:09:00

    @Jeffrey Zhao
    哦,明白了,是我第一个问题问的不好,其实我想问的就是多个线程是否会同时执行多个IHttpHandler(也就是是否会同时处理多个请求),现在看来这对我是个很严重的bug,因为我刚好使用了RequestContext.RouteData.DataTokens来传递namespace..

    也就是说有可能多个用户同时请求不同area的action,有可能导致返回不是他们所希望看到的area相关action

  10. 老赵
    admin
    链接

    老赵 2009-08-18 17:13:00

    @SoJin
    那对你来说还真是问题了……哎,其实我目前也正在做类似的事情呢。
    我就是做了个约定,如果area是Xyz,那么就去找Aaa.Bbb.Xyz.XxxController,否则就直接去找Aaa.Bbb.XxxController。

  11. SoJin
    *.*.*.*
    链接

    SoJin 2009-08-18 17:17:00

    @Jeffrey Zhao
    那您晚上有空的时候最好看下我上周给你发的那个疑问,是相关area的另外一个问题。可能会带给你更多惊喜。。。(再次感叹,”这就是微软的东西啊“)

  12. 老赵
    admin
    链接

    老赵 2009-08-18 17:20:00

    @SoJin
    我晚上看看吧。
    // 我倒觉得没必要妖魔化微软,谁的框架没有bug,只是我们刚好用了所以遇到了而已。

  13. Cat Chen
    *.*.*.*
    链接

    Cat Chen 2009-08-18 17:29:00

    SoJin:
    请问下老赵,IHttpHandle同一时间会有多个实例同时执行吗?还是说每个请求都会等上一个请求执行完才会执行?
    上次看mvc源码也“似乎”发现过这个问题,但有人告诉我同一时间只会执行一个请求,所以我就认为这个不会有影响。

    另外既然老赵看到了DefaultControllerFactory.GetControllerType
    方法能否再花点时间解答下我一周前短消息你的那个关于mvc area的疑问呢?




    IHttpHandler有一个属性IsReusable,如果它返回true就复用同一个实例,如果它返回false就不复用。但是,在Vista IIS7 integration mode里面,这是有问题的,就算你返回false它还是复用同一个实例。

  14. 老赵
    admin
    链接

    老赵 2009-08-18 17:34:00

    @SoJin
    对了,其实一个ControllerFactory很容易写的阿,例如这是我的代码,我写这个也就花了半个小时时间。

    public class AreaControllerFactory : IControllerFactory
    {
        #region IControllerFactory Members
    
        private ReaderWriterLockSlim m_rwLock = new ReaderWriterLockSlim();
        private Dictionary<string, Type> m_controllerTypes = new Dictionary<string, Type>();
    
        public IController CreateController(RequestContext requestContext, string controllerName)
        {
            Type controllerType;
            object area;
            if (requestContext.RouteData.Values.TryGetValue("area", out area))
            {
                controllerType = this.GetControllerType(area.ToString(), controllerName);
            }
            else
            {
                controllerType = this.GetControllerType(null, controllerName);
            }
    
            if (controllerType == null)
            {
                throw new HttpException(404,
                    String.Format(
                        "Controller of path {0} not found.",
                        requestContext.HttpContext.Request.Path));
            }
    
            try
            {
                return (IController)Activator.CreateInstance(controllerType);
            }
            catch (Exception ex)
            {
                string message = String.Format("Error creating controller {0}" + controllerType);
                throw new InvalidOperationException(message, ex);
            }
        }
    
        private Type GetControllerType(string area, string controllerName)
        {
            string namespaceBase = "MyApp.Web.";
            string typeName = String.IsNullOrEmpty(area) ? 
                namespaceBase + controllerName + "Controller" :
                namespaceBase + area + "." + controllerName + "Controller";
    
            Type type;
    
            this.m_rwLock.EnterReadLock();
            try
            {
                if (this.m_controllerTypes.TryGetValue(typeName, out type))
                {
                    return type;
                }
            }
            finally
            {
                this.m_rwLock.ExitReadLock();
            }
    
            type = Type.GetType(typeName, false);
    
            if (type != null)
            {
                this.m_rwLock.EnterWriteLock();
                try
                {
                    this.m_controllerTypes[typeName] = type;
                }
                finally
                {
                    this.m_rwLock.ExitWriteLock();
                }
            }
    
            return type;
        }
    
        public void ReleaseController(IController controller)
        {
            IDisposable disposable = controller as IDisposable;
            if (disposable != null)
            {
                disposable.Dispose();
            }
        }
    
        #endregion
    }
    

    反正是自己项目用的代码,就可以自己制定“约定”,不需要的功能也可以不要。
    对于这样一个东西,我觉得要么去修改它的代码并编译(开源的),要么自己写一个简单的。想要去使用trick来“迎合”它反而累。

  15. 老赵
    admin
    链接

    老赵 2009-08-18 17:38:00

    Cat Chen:
    IHttpHandler有一个属性IsReusable,如果它返回true就复用同一个实例,如果它返回false就不复用。但是,在Vista IIS7 integration mode里面,这是有问题的,就算你返回false它还是复用同一个实例。


    嗯,但是(我印象中)也不是像Single那样的复用法,而是等一个实例执行完了会再拿来用,对于单个实例来说还是单线程的。
    // IIS 7的问题我记得你写过文章的,呵呵。还好ASP.NET MVC是自己实现了一个类似HttpHandlerFactory样的东西。

  16. SoJin
    *.*.*.*
    链接

    SoJin 2009-08-18 17:38:00

    @Cat Chen
    !这表明在iis7 "integration mode"里IHttpHandler必须是线程安全了罗?要不就会有很多问题产生了。

    另外integration mode是什么东西?本人对iis并不是很熟悉。能简单介绍下或者给个中文介绍导航吗?

  17. Cat Chen
    *.*.*.*
    链接

    Cat Chen 2009-08-18 17:58:00

    @SoJin
    Integration Mode就是让IIS直接接管web.config,它自己按照web.config里面的配置来执行IHttpModule和IHttpHandler,不需要经过ASP.NET引擎。

  18. Cat Chen
    *.*.*.*
    链接

    Cat Chen 2009-08-18 17:59:00

    @Jeffrey Zhao
    这其实就是那个问题的workaround嘛,在用IHttpHandler的场合都自己封装一个IHttpHandlerFactory也可以,在Factory里面保证new Handler就行了。

  19. 老赵
    admin
    链接

    老赵 2009-08-18 18:01:00

    @Cat Chen
    确切地说,应该是IIS和ASP.NET引擎integrate在一起了。

  20. 老赵
    admin
    链接

    老赵 2009-08-18 18:01:00

    @Cat Chen
    不过我看asp.net routing倒应该不是估计来这个workaround的,asp.net routing设计的还是很不错的,虽然现在自带的功能往往不太够用。

  21. Cat Chen
    *.*.*.*
    链接

    Cat Chen 2009-08-18 18:05:00

    @Jeffrey Zhao
    我没有研究过Rails的routing,有空应该看看这两者的相似程度。MVC抄Rails的设计实在是太多了,就算是好的,也不敢保证是原创的。反而model没抄好,migration之类的功能没有,Linq to Sql又难以适应大项目。

  22. 菜鸟毛
    *.*.*.*
    链接

    菜鸟毛 2009-08-18 18:05:00

    老赵,我拜读你的MVC课程(27)《辅助普通Web应用程序开发(三)——Model Binder》之后,发现有些调试不通的地方,在webcast也找不到源代码下载,于是在自己修改了一番后,终于通过了.也能正常运行了,

    不过我认为这已经偏离了你的最初意图,例如IEnumerable<T>这个接口根本没有ForEach的方法.把它转为List<>后才可以,我也不知道你是如何实现的.还有辅助方法的调用方式,都不一样,请赐教啊.

    这篇文章地址是http://www.cnblogs.com/CoreCaiNiao/archive/2009/08/18/1549186.html

  23. 老赵
    admin
    链接

    老赵 2009-08-18 18:09:00

    @Cat Chen
    ASP.NET MVC就是个Web MVC,Rails是个整体开发环境。
    Migration我现在用Migrator.NET,感觉还是挺不错的。
    其实Rails也就是个ActiveRecord啊,感觉也受限很多。
    .NET上已经有了NHibernate,也有ActiveRecord,够用了。
    LINQ to SQL其实很适合小项目,非常非常方便,和NHibernate一个天一个地。
    其实.NET上感觉已经也不缺了,对于你我这种能够比较轻松地自己Hack现有代码的人,其实整个环境已经蛮完整了。

  24. 老赵
    admin
    链接

    老赵 2009-08-18 18:10:00

    @菜鸟毛
    自己写一个static void ForEach<T>(this IEnumerable<T> source, Action<T> action)啊。

  25. SoJin
    *.*.*.*
    链接

    SoJin 2009-08-18 18:14:00

    Cat Chen:
    @SoJin
    Integration Mode就是让IIS直接接管web.config,它自己按照web.config里面的配置来执行IHttpModule和IHttpHandler,不需要经过ASP.NET引擎。



    哦,谢谢

  26. 韦恩卑鄙
    *.*.*.*
    链接

    韦恩卑鄙 2009-08-18 21:17:00

    我在写游戏大厅的时候根本想不出怎么测试聊天室的并发 结果都是靠实际用户测试 在catch的log和无数次崩溃中修复的

  27. xland
    *.*.*.*
    链接

    xland 2009-08-19 08:24:00

    @Jeffrey Zhao
    老赵能否把你经常看的一些网站公布一下
    也希望老赵说一下自己的学习方法

  28. 老赵
    admin
    链接

    老赵 2009-08-19 09:13:00

    @xland
    网站全在博客的边栏里。

  29. 老翁
    *.*.*.*
    链接

    老翁 2009-08-19 09:36:00

    昨天把asp.net的页面类的继承机制顺了一遍,发现了很多以前不知道的事情,看来技术离不开刨根问底啊!老赵有没有关于Asp.net整个架构设计的书或者文章推荐一下呢?我想知道在页面上一个简单按钮点击服务器整个响应过程。

  30. airwolf2026
    *.*.*.*
    链接

    airwolf2026 2009-10-16 15:46:00

    一个小疑问,文章里面说'把对象作为方法的参数传递,可以线程安全'
    那应该这个方法本身是线程安全吧?

    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;
    }

    比如这个fix过的方法和原先的比,就差了赋值(写).光表面看不出问题

发表回复

登录 / 登录并记住我 ,登陆后便可删除或修改已发表的评论 (请注意保留评论内容)

昵称:(必填)

邮箱:(必填,仅用于Gavatar

主页:(可选)

评论内容(大于5个字符):

  1. Your Name yyyy-MM-dd HH:mm:ss

使用Live Messenger联系我