singleton concurrent access issue

[Walter Andresen]

Hi there!

I have a singleton object created based on a spring config file. Now someone used instance variables inside this singleton bean. In there applicatations, there are multiple sessions that use this bean and they corrupt instance variables, so it has inconsistent data

Is there any way to prevent this type of errors during development? Like code review?

would code analyzers help?

Thank you

[Craig Walls]

The best way to avoid anyone messing with the properties of a singleton bean like that is to (1) make them final and (2) not provide setter methods for them. Ideally, provide the values via constructor args and not by setters.

That said, I recognize there are cases where you need setter methods for non-final properties. In that case, it may be best to only inject those objects as an interface type that doesn't offer setter methods. Without those setter methods, it'd be difficult (not impossible) to change the values.

In the end, however, if members of the team are hell-bent on changing values that shouldn't be changed, the best way of dealing with it is with code review and frank discussion about why changing them is wrong. In short, although there are technical remedies, the real problem is a people problem, not a technical problem.

[Tim Holloway]

That's not really Spring's problem. What you need is synchronization.

While I think that Spring may have some synchronization support, your primary defence against concurrent modification is the synchronization services built into the Java language itself.

[Walter Andresen]

Craig Walls wrote:The best way to avoid anyone messing with the properties of a singleton bean like that is to (1) make them final and (2) not provide setter methods for them. Ideally, provide the values via constructor args and not by setters.

That said, I recognize there are cases where you need setter methods for non-final properties. In that case, it may be best to only inject those objects as an interface type that doesn't offer setter methods. Without those setter methods, it'd be difficult (not impossible) to change the values.

In the end, however, if members of the team are hell-bent on changing values that shouldn't be changed, the best way of dealing with it is with code review and frank discussion about why changing them is wrong. In short, although there are technical remedies, the real problem is a people problem, not a technical problem.

does it mean that the bean must tread safe? I am a bit confused. Thread safe means that multiple threads can access the bean at the same time and don't corrupt the data. But what if thread#1 changes varUser and then in 2 seconds thread#2 changes varUserJobTitle in the way that varUser and varUserJobTitle are not consistent. So we will have varUser=JamesBond and varJobTitle=ConstractionWorker. While all the methods are declared as synchronized.

[Tim Holloway]

Yes, it means that the object must be thread-safe. It is up to the object to ensure its own integrity by rejecting states where its internal data is inconsistent.

If you want to change multiple properties, it's generally better to synchronize the entire set of properties as a single transaction rather than doing them one at a time. You can to this either via a synchronized external mediator or by using something like a multi-argument synchronized method within the object.

[Craig Walls]

Walter Andresen wrote:does it mean that the bean must tread safe?

As has been mentioned, this isn't really a Spring question...more of a general Java question, so I'll toss one more response your way then move on...

Thread safety becomes a lot less important if the object in question is immutable. Who cares how many threads are accessing an object if none of those threads are allowed to change the object? They don't need to be synchronized or any such thing. Moreover, thread-safety and synchronization doesn't guarantee that a thread won't change a value that another thread is counting on...it only guarantees that they won't change it at the same time that the other thread is working with it (during the lock period). You could extend the lock period so that each thread has a full lock for the life of the thread, but then you will run into performance issues while all of the threads line up to wait for their turn.

Again, the best way to avoid concurrency issues is to employ immutability...that is, set those properties when the object is created (via constructor args) and don't offer setter methods where anyone can change them. Immutable objects are inherently thread-safe. But if immutability isn't an option, then judicious use of synchronization is okay...just be aware of the potential for performance problems.

[Raymond Tong]

If the class would be accessed as a Singleton, would it be better to use static variables instead of instance variables?
One possible way to catch this kind of problem is using static analysis tool like SonarQube.
Here is one of the rule specification you may be interested https://jira.sonarsource.com/browse/RSPEC-2213

By the way, there is some discussion saying Singleton is an anti-pattern.

Raymond

[Stephan van Hulst]

Never use static variables. The only things that should be static are types, constants and pure functions.

Configuration settings are variable. Even if they might not change during a run of the application, that's not to say they don't change between different application runs, or different systems.

Instead of creating a configuration singleton that you can access from anywhere, consider making it a regular class that you pass to the constructor of anything that needs the configuration. That way your program will be much more easy to test, and much more robust.

Singletons are an anti-pattern if they don't represent constant value types.

[Craig Walls]

In my experience, static variables are hardly ever a solution to anything. They seem tempting at times, but become painful as your application evolves. I like statics for constants, but that's about it.

I wouldn't go so far as to say that Singleton is an anti-pattern. It has its place.

And, to be clear, a Singleton bean in Spring isn't exactly the same thing as a Singleton type in Java. A Singleton bean means that Spring will create one and only one instance of the type with the given specifications...but others (including Spring) may create another instance of the same type with different specifications. This is definitely useful, because in most cases there's no need to have more than one instance of a component...and doing so would increase the classloading profile (in terms of memory used by each instance and instance-creation/garbage-collection) of the app. A singleton in Java means that there's one and only one instance ever.

The gotcha with singletons is that (as I've mentioned already in this thread) they probably should be immutable. If you keep request-specific data in a controller's instance variables, for example, then you're asking for trouble.

Of course, not all beans in a Spring container have to be singleton. The default is for Spring to create singletons, but you can specify other scopes, such as prototype (one instance per injection point), request (one instance per request), or session (one instance per session). And you can create custom scopes. But the reason for singleton being default is because, as I mentioned, most components don't need multiple instances and creating/maintaining multiple instances can have a negative performance impact. Just keep those singleton beans immutable and you'll be doing fine.

[Tim Holloway]

Here's an example of a singleton in Java that is anything but an anti-pattern: a Factory.

Factories are almost always singleton objects, frequently accessed via static methods, and very often are not immutable, especially if they maintain pools of whatever they produce.

For example, the various types of Spring Factory Beans.

[Craig Walls]

Tim: Good examples.

In any case, the debate over singletons and concurrency issues are really more general Java questions and not Spring-specific. Not being a regular around here, I'll let everyone else decide how they want to deal with this thread (IMO, it belongs in a separate forum), but going forward I'll dismiss myself from this discussion unless the question pertains to Spring or Spring Boot.