Syskiller–how to kill your system without notice
We have a bunch of smart people whose job description does not include breaking RavenDB, but nevertheless they manage to do so on a regular basis. This is usually done while subjecting RavenDB to new and interesting ways to distress it. The latest issue was being able to crash the system with an impossible bug. That was interesting to go through, so it is worth a post.
Take a look at the following code:
This code will your application (you can discard multi threading as a source, by the way) in a particularly surprising way.
Let’s assume that we have a system that is running under low memory conditions. The following sequence of events is going to occur:
- BufferedChannel is allocated.
- The BufferedChannel constructor is being run.
- The attempt to allocate the _destinations list fails.
- An exception is thrown upward and handled.
So far, so good, right? Except that now your system is living on borrowed time.
You see, this class has a finalizer. And the fact that the constructor wasn’t called doesn’t matter to the finalizer. It will still call the Finalize method, but that method calls to Dispose, and Dispose expect to get a valid object. At this point, we are going to get a Null Reference Exception from the finalizer, which is considered to be a critical error and fail the entire process.
For additional fun, this kind of failure will happen only in under harsh conditions. When the OS refuses allocations, which doesn’t happen very often. We have been running memory starvation routines for a while, and it takes a specific set of failures to get it failing in just the right manner to cause this.
Comments
This is a great test case for RavenDB and how clients can misuse/misunderstand
IDisposable
.Two significant examples of
IDisposable
misuse here:Dispose(bool)
.The C# tooling can help though: CA1063: Implement IDisposable correctly. And for folks that want a deeper treatise on best practices, I recommend bookmarking C# Finalizers and IDisposable for the situations when you truly need
IDisposable()
.Joe F, Note, however, that the class is trying to return buffers back, so they can be reused. That is a common enough scenario, and something that I do actually want the finalizer to do in this context.
I think this is mixing requirements and strategies to accomplish those requirements.
The requirement as I understand it is: "these buffers must be pooled and reusable". One strategy to achieve that is to misuse
IDisposable
as managed-RAII like this snippet tries to do. Another strategy is to create an interface for reserving and releasing these buffers.When a client uses the first strategy and takes a short cut by implementing less than half of
IDisposable
, don't be surprised when that short cut cuts back ;-PC# cannot support RAII by design, so if clients insist on
IDisposable
to approximate it, they also sign-up for the work of implementing the full pattern, which includes making the finalization order deterministic.Joe F, What is the interface for reserve / return that isn't the existing pool interface? The idea with such a structure is that you want to allow users to define explicit scope, and have a second time to recover from that, if they didn't.
I'd say this is a case-by-case design. Given that "A correctly-written program cannot assume that finalizers will ever run", it's critical to identify the full set of requirements for speed, memory, robustness, concurrentness, et al. For distributed and redundant server environments like RavenDB, you'll of course have a better understanding than me, a mere single-host device driver and application software engineer.
The only explicit scoping mechanism in C# is the
using
statement, so your solution space has a size of one: a full and correct implementation ofIDisposable
(unless you can do everything on the stack with value types 😂). For recovery and robustness, you can consider subscribing to the different global .NET framework events, like AppDomain.UnhandledException (and that page has links to other similar-in-purpose events).Lastly, for this kind of engineering problem, where you need to shepherd every byte immediately and safeguard against neglectful clients, I'd switch away from thinking in C# to thinking in C or C++. How would you implement RAII in those languages?
Herb Sutter summarized this as:
Joe, Ideal is nice, but isn't relevant. In practice, I can be sure that:
The key here is that finalizers may not run if you need them to verify behavior. For example, to release a lock, rollback transaction, etc. For resource usage, you can rely on them very well, because either the OS will clean them up or the finalizer will, which is what it is meant to do.
The
using
statement doesn't work here, in this case, we had a failure while constructing anIDiposable
instance to be place inusing
, so we have to have a finalizer to release these resources.Comment preview