HttpContext.Current is thread safe where a static member is not?

Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "hibernate.cfg.xml") looks like one...

It was probably the only "safe" way to get that path.

On of them might be using AppDomain.CurrentDomain.BaseDirectory to get the path of the executing assembly instead of Assembly.GetAssembly() or Assembly.GetEntyAssembly() and then figuring out the path based on their location.

I have hit a problem like that while unit testing a winforms app (entry< assembly was different of course, stupid me :-) ).

Why not just use CurrentSessionContext.Bind(..) and .Unbind(..) ?

You have changed this line:

.Configure(Path.GetFullPath(@".\hibernate.cfg.xml"))

to this:

.Configure(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "hibernate.cfg.xml"))

also the if(CurrentSession != null)

Putting things into constructor instead of application_start? don't know...

Ouch, ignore what I said. It is not a constructor.

One problem has to be a cast problem in the CurrentSession property getter:

return (ISession)HttpContext.Current.Items["current.session"];

This should be written as:

return HttpContext.Current.Items["current.session"] as ISession;

Your subtle bugs are enlightening and infuriating at the same time. Aside from the code to determine the path of the config file, nothing else seems like it could cause a problem - which means I'm missing a key understanding =(

@VirtualStaticVoid

You are wrapping your code in cotton wool. The code explicitly puts an ISession instance into the context at that key and therefore it is more than reasonable to expect an ISession instance to be retrieved from the same key.

Your solution also creates a nasty problem in that it hides the true error by causing a null ref exception at the first place the session is used rather than an invalid cast exception at the point the error could occur.

Why is "CreateSessionFactory" protected?

I've found that:

msdn.microsoft.com/.../....hostingenvironment.aspx

Is useful regarding reliably and outside of http context way to get the apps path.

(for reference)

I guess technically you could try and get the session before the BeginRequest has fired (or fired at all if it isn't a http thread).. and you assume a http context.. but those are things you could well accept if you dont spawn any other threads or try and use the session before the request as started (although perhaps ioc resolution happens before that).

I have no idea what the problems were, but I would assume it could be

creating the session factory in Application_start or on first use instead of static constructor

The Global method where the events get bound will never get called. It seems that it should be an instance ctor so that they get bound when the app starts.

Andy, there are multiple HttpApplication instances running on a website, they act almost as 'runners', and return to a pool where a set amount of them is kept alive, the ctor should run (wouldn't be subtle if it didnt I guess ;)).

One's got to be the Path.Combine, which is good practice any time you build up a path from multiple parts, since simple string concats can fail if slashes, etc. are/aren't provided when expected.

The second I'm not sure about, but I'd guess using AppDomain.CurrentDomain.BaseDirectory? I've seen a lot of code that just used "\" or similar, not taking into account that apps may be published in something besides a root directory.

When the protected Global is called ? I guess it is never called so the session is never created ?

public static ISessionFactory SessionFactory = CreateSessionFactory();

instead of a static constructor?

I think you might have changed CurrentSession.Close() into CurrentSession.Dispose().

It might have something to do with TransactionScope. With Session.Close(), NH may attempt to disconnect the connection while a transaction is still in progress... resulting in an ocassional exception.

I use CurrentSessionContext.Bind(..) and .Unbind(..) -- here's how:

zvolkov.com/.../...ASPNET-MVC-with-NHibernate.aspx

Off the top of my head, I'd guess that storing the CurrentSession in the request instead of the Session is a bug fix. One might assume that the session is safe because it'll get refreshed with each request. But then what happens with AJAX requests. Suddenly, your session drops from beneath you because of a concurrent dispatch.

The 2nd bug fix... just a guess.. but I think you changed NHinitialization from application_start (or init) into a static method. This is related to an issue in IIS7 where you can't initialize NH during application_start or init.

I think it's because there can be more than one Global instance.

I am not aware of the issue you brought up.

Not sure if this is a problem in NH, but we had that problem in SharpArchitecture (or rather Fluent-NH) with IIS7 where NHibernate.Init() crashes when called in application_start.

code.google.com/.../detail

So they changed it to singleton-method call in each Application_BeginRequest:

NHibernateInitializer.Instance().InitializeNHibernateOnce(

            () => NHibernateSession.Init(etc etc));

Anyway, maybe you changed it because you want it invoked only once... And we can't guarantee application_start to be called once in a lifeatime of an application, particularly in the case of application-pool recycling.

Do you need to set SessionFactory to null when the application ends to avoid memory leaks?

Application_Start is not viable as many instances of HttpApplication is created, the later ones will not benefit from this. Init may be used for that purpose, instead of Application_Start.

protected void Global() is not a ctor and this made confused :)

The only thing I can think of is that perhaps the static init could silently fail, causing you to wonder why you don't have a session factory for subsequent requests.

Meh, scratch that, aparently static init will def~ happen in context of the first request.. maybe related to static field init ordering?

I think it is impossible to look at working code and guess what the error was in the old code that you have not seen. There are millions of possibilities.

For example, I think the code was here...

You took out the NotImplementedException in CreateSessionFactory()

:-)

Stephen,

I thought the same, the silent fail thing,it happened to me several times during dev, I had to catch the exception in the debugger to find the problem.

I think the "bug" that I can spot, is that you are creating a session factory each time there is a webrequest rather than caching up the session factory in a static reference and checking to see if ts null or not.

No, the session factory is only created once.

OK, so.. is it that when running in classic mode (or IIS 6 and under), Application_Start will cause an exception only on the first request, but the next requests will all run and have no session factory (confusing to debug).. and the static initialization will throw every time a request tries to fire, thus making it easier to see what went wrong.

(Note that I think IIS7's default integrated mode now ensures the same for app_start).

Request isn't available for App_Start in IIS7

Also, you have a potential race condition under app_start if multiple requests come in. I'd assume however that IIS serializes/queues further requests until app_start completes. IF it doesn't though...none of the requests that came before App_Start completed will have the SessionFactory initialized.

ASP.NET uses a pool of HttpApplication instances, however Application_Start() method is called only on one of them. So when you bind events in the Application_Start() that the only one will have wired events and the others won't have wired events.

That is why you must use Global ctor.

You are using HttpContext.Current instead of [ThreadStatic] ISession property - this will prevent from various bugs that related to IHttpAsyncHandler.

Event handlers added in ctor will be fired before handlers registered via Global.asax (Application_BeginRequest method, Application_EndRequest and so on) and before handlers of modules. And, as I think, this is the only reason why you used this approach.

This is all I can see from this code at the moment.

And also you are using CurrentSession.Dispose() instead of CurrentSession.Close() because dispose method contains logic for logging of a session disposal. If you use Close method at the end of request information about session disposal will be logged only on run of GC. So the statistics will be wrong.

I seeing one issue more if you will use asp.net mvc BeginRequest will be called when you getting .css, jpeg.... files. And the application will open one session for each image or style file loaded.

It would be smart to only open it for aspx and other file extensions which will use nhibernate.

@Niclas, this isn't specifically an mvc thing.. whatever requests you map IIS to handle (and with IIS7 this is even more so) will get sessions.. you would potentially filter by the handler type, not the extension which is unreliable.. an easier system which has other benefits is to move such static content onto another 'dump' domain, so you can cut the request / response down to an absolute min.

One thing I was wondering ayende, was why you chose to store the nhibernate session in in the http session, couldn't you just have an instance variable, and assign to it on begin request and dispose of it on end request. I'm sure IIS ensures that a given HttpApplication can only handle one request at a time (sounds obvious but async handlers could mean this isn't so).. is it purely so you can use the static helper property to find the session?

Stephen,

I am not putting it in the http sesson, I am putting it on the http request

Ah yes, completely misread the code.. I always assume Session when I see default property access with httpcontexts.

Apologies!