I have a java file of the following format
class Demo {
private static Logger logger;
...
...
public static void main(){
// code to initialize logger
demoMethod()
}
private int demoMethod {
synchronized (tmpObj) {
...
helperMethod(tmpParam);
// some more logic of demoMethod related to file reading
logger.log("message from demo method");
...
//there is more logic that involves more loggers
}
}
private int helperMethod(int tmp){
//some logic of helperMethod related to Files
if(condition) {
logger.log("message from helperMethod");
}
return 1;
}
}
As you can see from the above blue print of code, I have loggers in both demoMethod( which has a syncronized block) and helpMethod. Now the issue I'm getting says
Unguarded read missing_lock: Accessing logger without holding lock TempClassName.tmpObj Elsewhere, logger is accessed with TempClassName.tmpObj held 8 out of 12 times.
I'm not quite sure of what is causing this issue, because the logger I'm using is thread safe. Can anyone please provide more insights to what might be causing this issue.
The primary reason for this report is: "Elsewhere, logger is accessed with TempClassName.tmpObject held 8 out of 12 times." That is, the member logger is accessed (read or written, but excluding initialization) at 12 textual locations in the source code, and for 8 of those locations, the tmpObject lock is known to be held by the executing thread. The tool has therefore concluded that the intention of the programmer is that tmpObject must be held in order to access logger. Evidently, 8 out of 12 is above the configurable threshold for regarding the association as more than coincidence. The tool therefore reports the deviations from the pattern as possible mistakes. I would naively expect there to be a total of 4 reports for logger, of which the one you have quoted is one (although it can get complicated by the existence of different possible call paths).
It is possible this is a false positive. This checker does not have a deep understanding of synchronization issues, it is merely extrapolating from a pattern it has discovered in the code. If, as you say, the Logger class is thread-safe, and there are no read/write or write/write hazards on the logger member itself, then it should be safe to dismiss this report as false.
Furthermore, note that in the code fragment you have provided, helperMethod is private, is only called once, and that call site is itself within a synchronized block. If the real code does not have any other call sites, then this might simply be a bug in the Coverity tool, since the access in helperMethod would always happen while the lock is held, and the tool should be aware of that fact.
But one more wrinkle: the message you quoted refers to tmpObject, while the code you posted refers to tmpObj, which are different names. Is this just a typo in your post, or does the real code actually have two different "temporary objects" running around creating confusion? If so, that's probably the real problem.
Details of this report aside, be aware that Java concurrency is much more difficult and subtle than it might seem at first. If you're not well-versed in the topic (for starters, do you have a solid grasp on "happens before"?), consult with someone who is or do some additional research before dismissing the report. This checker's understanding may be shallow, but in my experience it is often worthwhile to carefully review the use of concurrency in any code it flags.
Disclosure: I was previously employed by Synopsys/Coverity and helped build the Coverity tool.