When an exception is thrown inside a lock, should we release the lock?
Of course, if this exception is expected and we can recovery from it, we should perform proper actions to make lock protected data in consistent and release the lock. If the exception is unexpected, eg,., OutOfMemoryExceptions, LinkErrorExceptions, NullPointerExceptions, which might be thrown everywhere and we want to terminate the program to stop further data corruption. In this case, should we release the lock before terminate the program?
I prefer not release the lock. And there is a good post here:
Does a locked object stay locked if an exception occurs inside it?
I note that no one has mentioned in their answers to this old question that releasing a lock upon an exception is an incredibly dangerous thing to do. Yes, lock statements in C# have "finally" semantics; when control exits the lock normally or abnormally, the lock is released. You're all talking about this like it is a good thing, but it is a bad thing! The right thing to do if you have a locked region that throws an unhandled exception is to terminate the diseased process immediately before it destroys more user data, not free the lock and keep on going.
Look at it this way: suppose you have a bathroom with a lock on the door and a line of people waiting outside. A bomb in the bathroom goes off, killing the person in there. Your question is "in that situation will the lock be automatically unlocked so the next person can get into the bathroom?" Yes, it will. That is not a good thing. A bomb just went off in there and killed someone! The plumbing is probably destroyed, the house is no longer structurally sound, and there might be another bomb in there. The right thing to do is get everyone out as quickly as possible and demolish the entire house.
I mean, think it through: if you locked a region of code in order to read from a data structure without it being mutated on another thread, and something in that data structure threw an exception, odds are good that it is because the data structure is corrupt. User data is now messed up; you don't want to try to save user data at this point because you are then saving corrupt data. Just terminate the process.
If you locked a region of code in order to perform a mutation without another thread reading the state at the same time, and the mutation throws, then if the data was not corrupt before, it sure is now. Which is exactly the scenario that the lock is supposed to protect against. Now code that is waiting to read that state will immediately be given access to corrupt state, and probably itself crash. Again, the right thing to do is to terminate the process.
No matter how you slice it, an exception inside a lock is bad news. The right question to ask is not "will my lock be cleaned up in the event of an exception?" The right question to ask is "how do I ensure that there is never an exception inside a lock? And if there is, then how do I structure my program so that mutations are rolled back to previous good states?"
C++ RRID pattern in this case will not release the lock:
...
{
// the destructor will release the lock when
// we go out of this block
AutoReleseLock lock(&mLock);
...
// Exception thrown here.
foo();
...
}
...
If the thrown exception is caught somewhere, the lock will be released. Otherwise, std::terminate() will be called directly without release the lock.
But in Java:
synchronized (monitor) {
...
// Exception thrown here.
foo();
...
}
or
lock();
try {
...
// Exception thrown here.
foo();
...
} finally {
unlock();
}
The lock will be released no matter the exception is caught or not, is expected or not before we get to the unhandled exception handler. And other threads may read corrupted inconsistent data and perform unpredictable actions, for example, remove wrong files.
So, how can I get the similar behaviour as in C++ to terminate the program without release the lock on unexpected exceptions without a lot of ugly code?
update:
Be exception safe or kill itself to prevent data corruption are both good ideas. But permit unpredictable behaviour is not a good idea, even if it's rare.
But, be exception safe as holding locks may require a lot of tedious logic. For example, as OutOfMemoryException in Java.
synchronized {
nameSet.add(newPeople.name);
ageSet.add(newPeople.age);
jobSet.add(newPeople.job);
}
will become:
synchronized {
nameSet.add(newPeople.name);
try {
ageSet.add(newPeople.age);
} catch (OutOfMemoryException e) {
nameSet.remove(newPeople.name);
throw e;
}
try {
jobSet.add(newPeople.job);
} catch (OutOfMemoryException e) {
nameSet.remove(newPeople.name);
ageSet.remove(newPeople.age);
throw e;
}
}
And, still bugs here. We should determine whether the name/age/job is already here before we add it to the set.
Most systems expect OutOfMemoryException to be a program kill action rather than a general exception or we end up catching them almost every where. In this case, this kill action are not as clean as in C++ and will release locks to leave inconsistent data to be read by other threads. And other threads may persist this data to disk.
