The Inquisitive Coder – Davy Brion's Blog

I recently posted some code that i asked you to review.  When i posted it, the code had never even executed (that’s right, not even through a test) and i only thought it would do what i needed it to do.  I consider the actual implementation non-obvious (at least for those who don’t know the copy-on-write approach to avoid traditional locking) so i just wanted to hear some reactions to the code from people who didn’t knew the context.  I promised to do a follow-up post to discuss the code in its entirety so here it is.

First, i’ll show the whole class again:

Basically, the purpose of this class is to hold a set of ISessionFactory instances, each of which belongs to a particular tenant in a multi-tenant application.  Tenants can be added on the fly (without restarting the application) and when an ISessionFactory doesn’t exist yet for a particular tenant, it must be created when the first request for an ISession for that tenant comes in.  Obviously, access to the sessionFactories dictionary must be thread-safe since multiple threads will be reading from the dictionary as well as occasionally writing to it.

I considered 3 options to make sure access to the dictionary would be thread-safe:

1. Traditional locking (through the lock statement or the Monitor class)
2. Using the ReadWriterLockSlim class
3. Using the copy-on-write pattern

Traditional locking was quickly scratched from the list because that would require me to lock for every read of the dictionary as well as every write.  Now, pretty much every single request requires an NHibernate session which means that pretty much every single request results in a lookup in the sessionFactories dictionary.  If i need to lock for every read, this significantly hurts overall throughput of the system. 

The ReadWriterLockSlim might be a good solution here… after all, the short description of this class in MSDN says this:

Represents a lock that is used to manage access to a resource, allowing multiple threads for reading or exclusive access for writing.

Sounds like what i need, right?  But the thing is, i’ve never used the ReadWriterLockSlim class before and it hasn’t really gained my trust yet.  I know that’s a terrible excuse for not using it, but here me out.  While the ReadWriterLockSlim likely reduces locking overhead over traditional locking substantially, there still has to be some overhead for read operations, even if it is small.  In most situations, that small overhead wouldn’t bother me but in this case, that little overhead would be added to pretty much every single request in the system.  Now, writing to a dictionary implies that a new tenant has been added to the system.  In the context of this system, that’s not even gonna happen on a daily basis.  Hell, once a week is probably a best-case estimation and even that is highly optimistic.  So i really don’t want any kind of overhead on read operations when the write operation is only going to happen very occasionally.

That leaves the copy-on-write pattern.  I’ve used it before with success (though at the time, i didn’t know it was a known pattern) so this approach has already gained my trust.  It basically implies that we don’t do any locking on the read operations, but whenever a write operation occurs we copy the original set of objects, perform the write on the newly copied set and then set the reference of the original set to the newly created and modified instance.  During this whole time, every single read is safe.  Successive reads within the same logical operation however aren’t, so the following code would not be thread-safe:

Because there’s no locking on the reads, the code within the if-block could fail because the sessionFactories reference could be pointing to a new dictionary which no longer contains the element for that key. 

Of course, if you have frequent writes, the overhead of copying the set of objects every time you need to add/remove one might be bigger than you want, so this isn’t a pattern that you should use whenever you need to protect access to a shared resource. For this situation however, i think it’s ideal… though i’d obviously like to hear about better solutions

Now, let’s take a closer look at the pieces of code that perform the write operations.  First, adding a new ISessionFactory to the dictionary:

As you can see, the entire operation is put between a lock on the writeLock object instance.  The downside of this is that creating an ISessionFactory instance is an expensive operation, which means the lock will be held for a long time (could easily be one or more seconds).  Then again, i don’t anticipate this happening frequently so it’s not that big of an issue… especially since reads aren’t being blocked by this anyway.  This approach also prevents the creation of 2 ISessionFactory instances for the same tenant.  Well, unless i missed a bug here :p

Now, once the ISessionFactory instance is created, we create a new Dictionary based on the contents of the old one and then we add the new ISessionFactory instance to it.  After that, we replace the sessionFactories references with the new dictionary and from that point on, every read will use the new dictionary instance.  During this entire operation, no read operation was impacted negatively. 

Now lets take a look at the other write operation, removing an ISessionFactory instance from the dictionary:

The first if-check, which happens outside of the lock is a bug that i missed but that was pointed out in the comments of the original post.  If CreateSessionFactoryForCurrentTenant and RemoveSessionFactoryForTenant would execute concurrently for the same tenant, it’s possible that the ISessionFactory instance of that tenant is never removed from the dictionary (and also never disposed of…) since the check happens outside of the lock and could be executed before the ISessionFactory of the tenant was added to the dictionary.  In that case, the ISessionFactory instance would stay in the dictionary as long as the application stays up.  This is definitely a race condition that you want to avoid in every other situation though in this case, the odds that we’re simultaneously adding and removing the same tenant are slim to none.  Nevertheless, i don’t want to be accused of promoting race conditions so we’ll make the change anyway

Now, as you can see we once again create a new dictionary based on the previous one, then remove the ISessionFactory instance for the current tenant and then we overwrite the sessionFactories instance once again.

Finally, there’s the read operation that i specifically didn’t want suffering from locking overhead:

The only time this code will block is when a new ISessionFactory for the current tenant needs to be created.  Luckily, that only happens once for each tenant.  As i mentioned earlier in the post, using this pattern doesn’t guarantee that successive reads within the same logical operation are thread safe, so there is a bug in here.  If a tenant already has an ISessionFactory instance, it’s possible that the RemoveSessionFactoryForTenant method has been executed between the if-check and accessing the ISessionFactory based on the tenantId.  In that particular scenario, the ISessionFactory instance is no longer in the dictionary which will cause this code to throw an exception.

That’s a bug that i don’t feel like fixing though… Once a tenant has been removed, they are no longer a paying customer.  If they are no longer paying for the software, there is no reason whatsoever why i should care about any possible exceptions they could get while running the software

Seriously though, if the RemoveSessionFactoryForTenant method is called, users of that tenant won’t even have access to the system anymore so it’s really a non-issue.

Anyways, i think i’ve covered the implementation in more detail than you probably cared for.  So, any thoughts? Are there still issues that i haven’t thought of? Is there another approach that you would use for this specific scenario?

Bram De Moor, one of my coworkers, has developed a small game which is a part of a university study. I can’t tell you what the study is about, since that would ruin the purpose of the game (and if anyone leaves a comment mentioning it, it will be deleted) but it’s pretty interesting. The game itself is pretty nicely done, though you obviously shouldn’t expect too much. So if you can spare about 15 minutes and want to participate in this study, download the game here, play it and answer the questionnaire at the end. Don’t do it for yourself, do it for science!

As i’m sure most of you have noticed, Ayende recently started an NHibernate Donation Campaign.  I was against the idea from the beginning and even though this post will probably piss off a few people (like i ever cared about that, right?), i wanted to share a few of my thoughts on this with you.

The idea of sponsoring an open source project and helping out financially is definitely nice, but there are some very important practical issues that need to be kept in mind.  Those practical issues are IMO being ignored by this campaign (which honestly feels like a publicity stunt more than anything else) … some of them were mentioned in the original mailinglist discussion but no resolution has been offered.  There are probably more issues that we haven’t even thought of yet, but the most important ones are (again, IMO):

• Who’s going to decide how much of the money will go to the committers, and what will that be based on?  Will it be based on past contributions? Future contributions? The complexity of the contributions? Total number of contributions? Total time spent on the project?  We can’t even measure some of these things, so this is bound to either raise some discussions sooner or later, or might even upset some people.  I can certainly imagine that some contributors from the past (like, the ones who did most of the really hard work in the beginning) could feel that they’re entitled to some (or most) of the money.   Discussing how to spend the money on committers is just going to open a can of worms that could possibly cause much more problems between committers than it would solve. 
• There has to be some legal entity (or at least a person) who’s in charge of the bank account where the money will be deposited.  At this point, we have no formal structure or organization to deal with something like that.  I’m no expert on legal matters at all, but i can’t help but think that this isn’t something you can just decide on in a mailinglist… Some level of formalization is most likely required here.
• Taxes… if the money goes to committers, then they will have to pay taxes on it based on the laws of the country they’re living in.  Tax laws obviously vary from country to country, but i don’t think it’s unrealistic to claim that between 25% and 50% of the money will simply go to the government(s) instead of actually benefitting the project.  You might wanna keep this in mind before you donate…

These are just 3 big issues that could come of this, and i don’t really see a true benefit to the whole thing.  The people who in the past have spent time on NHibernate either volunteered to do so, or were paid by someone (or some company) to spend a certain amount of time on it.  Without donations, it will just go on like that and i don’t really think there’s anything wrong with that.   While resources are certainly limited, this approach probably causes less problems than one that distributes donations amongst committers.

Now, just to be absolutely clear on this… i am one of the NHibernate committers, and i don’t feel that i’m entitled to any of the money that will be raised.  For one, i haven’t committed anything in the past 6 months, and the things i did commit definitely aren’t worthy of a donation.  If we are going to accept donations and spend it on people instead of infrastructure, i believe any money that would be raised should go to the people who originally started NHibernate and worked hard to get it to a state where it actually became usable in the .NET world.  If it weren’t for their efforts, it never would’ve been in a position to become the de facto ORM in the .NET world, regardless of who’s working on it right now.

I just wrote some code, and i’d like your opinion on it.  The thing is, i’m not going to provide any context as to what it does or why certain decisions were made since i know you guys like to be challenged.  You also might want to keep the following in mind when reading it:

1. it might contain bugs that i’m not aware of
2. it contains weird parts that were either brainfarts on my part, things i did on purpose to avoid possible issues, or both
3. it contains at least one bug that i know about, yet don’t care about
4. i removed the comments that i originally put in it to make things more interesting
5. i haven’t tested the code yet
6. i haven’t even executed it yet
7. i think it’s ok… but i’m not sure

Questions will be answered if you have them (and i’m sure you will) though i can’t make any promises as to how soon i can answer them… I will post a follow-up post to discuss the code in its entirety later on, though i’ll probably wait a few days to do so.

This is the code:

Ayende recently wrote about how most software development processes ignore the actual building of software.  Which reminded me of an excellent post written by James Shore over a year ago, and that i commented on as well.  What James (and Ayende) are saying is that if you don’t actually take care of the way you build the software, no process can guarantee that any kind of perceived progress in the beginning can be sustained over the long run.  Sustainable progress takes effort, and you’ll never keep it up if you don’t put the proper practices in place to enable it.

The sad (and typical) part is that the methodology which makes everything sound easy and prescribes the least amount of effort required is the one that took off the most.  Obviously, i’m talking about Scrum.  Scrum makes no mention whatsoever about technical practices that are required to enable sustained progress and Scrum proponents will proudly proclaim that it doesn’t need to since it’s about product development instead of software development.   You can easily become a certified Scrum master while you might not know anything about real-world software development.  In fact, i’ve seen plenty of people boasting that certification on their resume and their linkedin profiles while they really shouldn’t be involved in any kind of professional software development to begin with.

And since Scrum is simple enough for middle-management (you know who i’m talking about… the ones that are usually not competent enough to promote to upper management) to understand, they jump on it as well and before you know it, they’ll be talking about the Scrum concepts as if they actually knew what they’re talking about.  Teams and even entire departments in large corporations are told to be ‘agile’ and follow the Scrum rules and concepts for every new project.  And they do.  And it works great… for a while.  We all know what happens after that.

Then take a look at Extreme Programming (XP).  It’s been around for a while now.  And it unfortunately doesn’t have a good, catchy name.  In fact, its name is probably one of the biggest reasons why it never took off the way Scrum did.  XP prescribes a lot more than Scrum, and even details the technical practices that you’ll need to enable sustainable progress.  And while you don’t need every single one of them, they all provide major benefits and the more of them that you do use, the better off you’ll be.   So what is the problem with XP?  I never really understood why so many people jumped on Scrum, and now Lean (until the Toyota recalls that is) but never really got into XP.   Well yeah i do understand.  XP takes more work and effort to understand and apply.  And that, my friends, is the price you’ll have to pay for sustainable progress(*).

*: i’m talking about the actual effort and work, not XP in itself

If you’re using NServiceBus together with your own instance of Castle Windsor, there is one thing you really need to look out for.  NServiceBus was originally developed with Spring.NET as its IoC container, but it’s been changed to support multiple containers in a similar manner as Agatha does it.  Agatha however, was originally developed with Castle Windsor as its IoC container, and as such is well aware of Windsor’s need to explicitly release resolved components.   NServiceBus unfortunately was not aware of this need, and a workaround that they have introduced is to set Windsor’s ReleasePolicy to the NoTrackingReleasePolicy (which doesn’t hold any instances in memory, but doesn’t provide any cleanup either) if you configure NServiceBus to use its own instance of Castle Windsor.  However, if you’re integrating NServiceBus into a project that is already using Castle Windsor, then you probably want NServiceBus to use your instance of Castle Windsor.

And that is when problems might appear.  If you’re using Castle Windsor with the default ReleasePolicy (which is the LifecycledComponentsReleasePolicy) then each resolved transient instance will be stored in memory by that policy until the instance is explicitly released.  The benefit of this policy is that the container can automatically dispose any disposable transient dependency of a resolved component.  In my case, i’ve come to rely on that feature to achieve deterministic disposal behavior throughout my code base. 

Now, when you configure NServiceBus and pass it your instance of Castle Windsor, it obviously doesn’t change the ReleasePolicy like it does when it creates its own instance of Castle Windsor.   This is good because changing the policy of a passed in container would almost certainly have a huge behavioral implication for the application and such an action would be unacceptable when integrating a new framework into your project.   But since NServiceBus doesn’t have the notion of needing to release resolved components, every single transient instance it resolves through your container will be stored in memory until the application’s process is terminated.  Which means that you’ll leak instances of the following types for each message that your system needs to handle:

• NServiceBus.Grid.MessageHandlers.GridInterceptingMessageHandler
• NServiceBus.Sagas.Impl.SagaMessageHandler
• Your own MessageHandlers (and their transient dependencies as well)

If your MessageHandlers don’t have dependencies (highly unlikely if you’re already using an IOC container) then you’d still have 3 leaking instances per incoming message.  Add the number of transient dependencies of any handler to that and the number of leaking instances can increase dramatically.

First of all, if you do not depend on Windsor’s default ReleasePolicy’s behavior, then the easiest way to avoid this problem is definitely to set the container’s ReleasePolicy to the NoTrackingReleasePolicy like NServiceBus does itself when it’s configuring itself with a new instance of the container.

If you do depend on it, and you don’t want leaking instances that hang around forever, then the approach listed below is one way to solve this problem.  There are probably other solutions available, and while the approach listed below can definitely be considered to be a HACK, i do think it’s the best solution to this particular problem.

Because we don’t want to change anything about the way that we’re already using the container, we just need to make sure that NServiceBus’ usage of the container doesn’t conflict with ours.  That basically means that we need to make sure that the components that it resolves should not be tracked by the container.   There are 2 kinds of components that NServiceBus will resolve during normal operation:

1. Its own components that it needs to implement the features it offers you
2. Your message handlers that you need to handle incoming messages

The first category is easy to exclude from Windsor’s tracking behavior.  We can simply create our own ReleasePolicy which extends the default ReleasePolicy, and make sure that any instances of an NServiceBus-type are no longer tracked by the container:

Then you need to set this release policy somewhere in your application’s startup routine:

(in this case, IoC.Container returns an IWindsorContainer instance)

This will make sure that no instances of NServiceBus-types will ever leak in the container.  But now we still have to deal with our message handlers.  The solution to making sure they are not disposed is not the cleanest out there but hey, it works.  I basically introduced a base class that all my message handlers will need to inherit from, with the following implementation in the Handle method:

Uh oh… i just felt a great disturbance in the Force, as if tens of voices suddenly cried out in terror about my direct usage of the IoC container in a component!

Conceptually, this is wrong on many levels.  Then again, this makes sure that the message handlers (and their dependencies) that are resolved by NServiceBus will always be guaranteed to be released by the comtainer.  It might not be nice, but it avoids the memory leak and it doesn’t force me to change my other code.

And once NServiceBus is modified to release the components it resolves (if it’s ever modified that is…) i only need to get rid of my custom policy and the try/finally block.  Unfortunately, my existing message handlers will then all implement the Process method instead of the Handle method but that is quickly fixed with a simple rename-refactoring.

Even though this is a pretty big hack (IMHO), at least its impact on the code is minimal… it will be easy to get rid of once the real problem is solved in NSB, and there are no real downsides to this that i can think of at the moment.

I’m not sure if this is related to the time of the year, but i (and undoubtedly quite a few of you as well if you’re honest) am once again getting pretty annoyed with the fact that the blogosphere is once again filled with “you gotta do this or you’re just not cool”-kinda posts. The weird part is that the last time i complained about this was pretty much exactly a year ago.

These days, you’re an absolute moron if you’re still using a relational database. And forget about getting invited to the best parties if you’re not developing according to the Command and Query Responsibility Segregation (CQRS) pattern. And if you’re not doing Lean then you’re really not worth listening to. Oh no wait, talking about Lean isn’t hot anymore since the Toyota recalls so we can scratch that off the list, sorry about that one folks.

Now, i’m not saying that the whole NoSQL and CQRS thing is worthless. I actually find them both very interesting. But to all of you who are now pushing these things as if they are the greatest things since sliced bread, i would just like to ask one thing: can we get some more real-world examples? You do use these things in real-world projects before proclaiming that these approaches will fix all (or most) of our current problems, right? You’re not just trying to be cool on the internet, are you? Honestly, of all the people that i hear talking about these things lately i only really believe about a handful of them when they say they use these things in real world projects. I’ll leave it to you to figure out who the real ones are, and who is just jumping on the bandwagon.

And no, i’m definitely not going to claim that using an RDBMS for every system is the best choice. I complain about databases every day, but i’m not just going to claim that going with a NoSQL database is going to solve all our problems either. I’d actually feel rather silly doing so, unless of course i’d be able to actually back up what i’m saying. Which i haven’t really seen happen yet.

The thing is… there simply is no one-size-fits-all approach to software development and it’s pretty sad to see that some idiots actually appear to believe that there is. Sometimes an RDBMS makes sense, and sometimes it doesn’t. Sometimes MVC makes sense, and sometimes it doesn’t. Sometimes DDD makes sense, and sometimes it doesn’t. Sometimes CQRS makes sense, and sometimes it doesn’t. That goes for every tool, library, architectural style, or whatever other buzzword you can think of. Use what makes sense for what you need, and let’s not pretend to be someone or something that you’re not, ok?

I just had to write the following code:

and then this:

to make a problem go away.

Can you come up with a WHY comment? (the category of this post is another hint)

I’m still working my way through my backlog of unread posts in Google Reader (not even halfway through it :s) and one thing i noticed by reading some posts is that we (.NET developers) generally seem to be pretty loyal when it comes to which IOC container we use. Some of us might switch between different containers from time to time, but i get the feeling that most of us typically stick with the container that we’ve come to know the best.

Some people will stick with Castle Windsor, some swear by StructureMap, some will always use Autofac, some only want NInject… hell, i even know a few people who still prefer Spring.NET. The only one that doesn’t seem to have many loyal followers is Unity . Every container has its strengths and weaknesses but most of them range from very good to downright excellent. When new features are introduced to one container, they often quickly find their way into the other containers as well. A cool new feature in container X is rarely a good incentive to switch containers because odds are high that your favorite container will get the new feature pretty soon as well.

What i think is a big reason of this (observed) loyalty is that the more you rely on your IOC container’s power, the less likely you are to switch to another container out of fear that some things might behave differently or might not work the way you think or expect they will. If the container you’ve chosen to use has proved itself to you in production usage, you are very unlikely to replace it with another. After all, it’s a very important piece of how your project works and you probably depend on it a lot. You’ve probably become very familiar with its behavior in different circumstances and you’ve either come to rely on that, or have learned to live with it in some way. But you’ve learned how it works and you’re pretty sure that there are no unpleasant surprises that will pop up a couple of days or weeks after a new deploy. Because of that, i really wouldn’t feel all that comfortable with switching to a new container. And lets face it, they all have their little special cases that you really need to know about to avoid problems.

And if you are using features that are unique to your container, then you’re obviously even more unlikely to make a switch. In my case, i used Castle Windsor when i first started to use an IOC container. I ran into some issues with memory usage because of the way Windsor works, but once i learned that i always have to explicitly release any resolved components, the problems went away. In fact, i’ve come to rely on the fact that by releasing a resolved component, its disposable transient dependencies will automatically be disposed by Windsor as well. I’m not sure about this, but i think Windsor is the only container that does this, or at least one of the few that do. I actually consider it a must-have feature for a container, but most people don’t seem to consider this as important.

I can’t really imagine a situation that would make me consider switching to another container at this point… well, unless its development would someday halt obviously. And even then i’d probably be holding on to it for a long while. How about you? How likely are you to switch to a different container? And what would make you switch? What holds you back from switching to a different one? Which one do you use and why do you like it so much?

Ok, i’m back from my trip to New York. The picture above shows me and the 2 friends that joined me on the trip on top of the Rockefeller building. For those of you who have no idea what i look like (and i’m assuming that’s most of you since i never put a picture of myself on this blog before), i’m the dude on the left (_your left_). The entire trip was simply fantastic… we did so much stuff that we often didn’t remember what we did over 2 days ago without pulling out the digital cameras, but some of the highlights include:

• The statue of liberty… it’s a bit cliche, but seriously, the level of detail on that statue is truly astonishing… the French really did a fantastic job on it (yea i said it )
• Going to a taping of David Letterman’s Late Show… i was sorta hoping in advance that we could get in, but we really didn’t plan on it. We went into the Ed Sullivan’s theater asking for tickets, and luckily we were ’selected’ (the fact that it was freezing and that this was the tourist low season obviously helped) to attend a taping a few days later.
• Brooklyn Dekker… anybody who knows me in real life knows i have a soft spot for good looking women, but when Brooklyn Dekker (the Sports Illustrated Swimsuit Edition cover model) got on stage during the taping of the Late Show the entire audience kinda got silent for about 2 seconds.. she really looked that stunning.
• I’v been an NBA fan for the last 16 years or so… i finally got my first chance to see an NBA game in real life. Ok, it was the Kincks vs the Kings (2 of the worst teams right now) but it was still a great experience… and i was actually at Madison Square Garden! (if you don’t know the significance of that, don’t worry… only basketball fans will understand)
• My pancakes with strawberries and ice cream breakfast… yeah seriously, it was that good
• Me asking an NYPD officer holding an M16 politely if i could take a picture of him and answering “oh… ok then” when he said he didn’t want me to take a picture of him
• Walking through Brooklyn during Snowmaggedon looking for Grimaldi’s Pizzeria only to (finally) found it being closed.

Those were the highlights but pretty much every single moment of the trip was memorable and i truly hope that i never forget a single one of them. I took 1125 (according to iPhoto) pictures and they were definitely needed to remember everything that happened in such a short timespan. We went during the low-season and even then, New York is definitely worthy of visiting. The only thing you’ll regret is that you either didnt’ stay long enough, or that you didn’t buy enough clothes (thank you oh horrible Dollar).

Once i got back to my country (Belgium for those that don’t know), i was astonished to see how badly it was handling the less than one inch of snow after what i had just seen in Manhattan. The difference really is huge. I usually avoid politics on this blog as much as possible, but the way our government is handling things compared to how they’re dealt with in New York in much worse weather conditions is truly saddening. Less than an inch of snow and the whole country seems to be at a stand still. And people wonder why i dont go out to vote (that’s right… we are required to show up at elections but i refuse to show up because there really isn’t even a single person who’s worthy of a vote right now).

Anyways, i’m back and i’m working through the backlog of unread posts in Google Reader… there have been a couple of posts so far that might inspire a few posts but other than that, i’m lacking any kind of inspiration to write a real post at the moment so if you have any requests, now is the time to make them

To wrap up this post: it was great to be gone for a while, but it’s great to be back as well