As a Baeldung editor, I had the pleasure working with an author on an article on Common Concurrency Pitfalls in Java. This is a great read, but assumes a certain amount of competence on the part of the developer.
There are a couple of things I’ve seen which are instant concurrency fails. They’re easy to add to the code, and guaranteed to give you weird results. The fact that developers still commit these is a comment on how we educate them about OO and concurrency, which are very dangerous when used incorrectly.
A Code Review Aside
As a code reviewer, I’ve developed a few shorthands over the years. These help me spot areas to look at in more detailed in a big code change. They include red flags things I expect to go wrong. It’s a good idea to train oneself to spot key anti-patterns, or potential anti-patterns, as they can be valid code, but result in invalid behaviour.
Request State in Bean
In a Java application, the services, controllers, handlers and repositories are generally singleton. They’re created as the app is started, and then requests pass through them, often from multiple threads.
Consider code like this:
1 2 3 4 5 6 7 8 9 | public void processOrder(Order order) { ... currentLineItem = order.getLine(0); processLineItem();}private void processLineItem() { myService.store(currentLineItem);} |
In this, the author of the class has decided that the object can remember the item it’s presently working on, to save effort in terms of passing that item down to the next function.
This violates two principles: thread-safety and meaningful object state. It’s very unlikely that an order processor is really about the order it’s processing. You might imagine something that statefully iterates through an order, some sort of cursor, reader, or builder, but blending all of those together in a single object is muddy.
Most importantly, though, there’s a clear definition of why this is wrong. If every the attribtues of a request get put into the receiver of that request, then you have two risks:
- Bleed between requests in a multi-threaded execution
- Bleed between requests in single-threaded if things aren’t fully tidied up
In short, never do it!
Crazy Lazy Initialization
Lazy initialization allows for:
- Faster startup because of
- Just-in-time loading of resources when needed
- No loading of a resource if it’s not needed (example, a serverless Lambda which may never be asked to do a certain code path in its lifetime)
- Customisation of how a resource is loaded by activities that happen sooner
All of these are good. However, this code:
1 2 3 4 5 6 7 8 9 | private LazyService getLazyService() { if (lazyService != null) { return lazyService; } LazyService newLazyService = connectToLazyService(); registerWithServiceRegistry(newLazyService); lazyService = newLazyService; return newLazyService;} |
Though it will work, it can be called concurrently and will go wrong. How wrong it goes, depends on all sorts of things. In the example, I’ve tried to hint at the sorts of things we’re dealing with:
- In concurrent calling, more than one lazy-load happens…
- … if this is expensive, it’s a waste
- If more than one lazy-load happens, perhaps two objects stay resident in memory for longer than needed, or forever
- If this is meant to be a singleton, perhaps the request that gets the orphaned object somehow fails to coordinate with the rest of the requests
- Using a hand-made non-thread-safe object initialization is a real pity
For correct singleton initialization you should use double-checked locking or use a framework, or even judicious use of simple Java singletons based around static fields.
Other Concurrency Fails
The above two seem the most common things to get so wrong that it should be obvious. If you spot another, then please drop it in the comments.
Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: Two Common Concurrency Bugs Opinions expressed by Java Code Geeks contributors are their own. |
private LazyService getLazyService() {
if (lazyService != null) {
return lazyService;
}
LazyService newLazyService = connectToLazyService();
registerWithServiceRegistry(newLazyService);
lazyService = newLazyService;
return newLazyService;
}
from where LazyService comes from
In this example,
LazyServicewas an object being lazy-loaded as a singleton. A made up object/type.Hi…
I’m Elena gillbert.Researchers have spent a great deal of time and effort looking into concurrency bugs over many years. Much of the early work focused on
deadlock, a topic which we’ve touched on in the past chapters but will
now dive into deeply [C+71]. More recent work focuses on studying
other types of common concurrency bugs (i.e., non-deadlock bugs). In
this chapter, we take a brief look at some example concurrency problems
found in real code bases, to better understand what problems to look out
for. And thus our central issue for this chapter: