ninject / Ninject.Web.Mvc

extension for ninject providing integration with ASP.NET MVC
http://ninject.org/
Other
238 stars 85 forks source link

Untitled #11

Closed yads closed 13 years ago

yads commented 13 years ago

I had encountered an issue with the OnePerRequestModule where its EndRequest handler was running before my EndRequest handler. My EndRequest handler was ending my NHibernate UnitOfWork. However, since my NHibernate session was bound in RequestScope, my handler was getting a brand new session and was unable to commit the transaction. This was because OnePerRequestModule was running the DeactivateInstancesForCurrentHttpRequest method first.

remogloor commented 13 years ago

As the execution order of the modules is not really reliable, things that need to be done at the end of a request on an object in request scope should be done either in its dispose method or in a deactivation action. Therefore I think this change is not nesessary.

yads commented 13 years ago

While I agree with you that cleaning up objects that are bound to request scope should be done in the dispose method or in a deactivation action, I disagree that the order is not reliable. The order is the same order you register for the events. So this change allows modules to register for the EndRequest event and do things with the objects bound to request scope (eg logging) before they have been removed from scope.

remogloor commented 13 years ago

I still think all actions with InRequestScope bound objects even logging before deactivation should be done in a deactivation action or in dispose. Because this is exactly the time when you want to do thinks like this. Doing it in the EndRequest mixes up two different lifecyle management things. Actually, I do not see any situation where you want to access these objects in the EndRequest of a module. So there is no need for this change in my opinion. But probably I just don't see any advantage of using a module rather than a deactivation action.