Declaring variable final and static

Question

This comment was made in a code review and the person who made it is no longer on our team.

Any type that must be resolved by the classloader at runtime should never have instances which are held by references declared to be both final and static.

Here's the line of code:

private final static Logger log = LoggerFactory.getLogger(MyClass.class);

I'm familiar with the debate of declaring loggers static or non-static, but this comment seems to be more general. I can't find any explanations of why static and final are bad. Can somebody elaborate?

There is a good reason why he is not in your team anymore ... what you have is perfectly fine !!! There is nothing incorrect about it. — StackFlowed, Oct 07 '14 at 17:34
Probably the timing is wrong? Would this force the program to try to initialize `log` before `LoggerFactory` is ready? Sorry, I'm not clear on the order in which things happen in a Java program. — ajb, Oct 07 '14 at 17:34
"Any type that must be resolved by the classloader at runtime",well MyClass.class is perfectly resolvable at compile time ,otherwise it would have given compile time error.Note,you are using a class literal and not using something like this LoggerFactory.getLogger(Class.forName(String classname)) — Kumar Abhinav, Oct 07 '14 at 17:37
There are a lot of potential gotchas with static members and initialization, but OP's code looks OK. Reviewer's comment seems invalid. — FrobberOfBits, Oct 07 '14 at 17:38
Staring at the code in my project right now and we are using `private static final Logger`. So if you're doing it wrong, we're all doing it wrong. Oh, and a [relevant SO answer](http://stackoverflow.com/questions/6653520/why-do-we-declare-loggers-static-final). — Compass, Oct 07 '14 at 17:40
Maybe he had a bug in the code which he then assumed that "final static" was a problem, when in reality he had another issue.... — Petro, Oct 07 '14 at 17:45

score 13 · Accepted Answer · edited Oct 07 '14 at 18:07

13

The comment is most likely related to a problem of Classloader Leaking (here is a good article).

In a nutshell, this problem happens in environments where classloader needs to be reloaded. If you load a class dynamically through a classloader and then try reloading the classloader, keeping static final fields with objects of classes created through this classloader will prevent unloading the classloader itself. Once this happens, you get an OutOfMemoryError.

The article linked above lists logging libraries among the top culprits that could produce this behavior, along with measures you can take to work around the leaks (such as releasing classloaders explicitly).

edited Oct 07 '14 at 18:07

skuntsel

11,624
11
44
67

answered Oct 07 '14 at 17:53

Sergey Kalinichenko

714,442
84
1,110
1,523

We had discussions in the past about declaring loggers static and issues that may have on class reloading, so I suspect that was partially his concern here. – sdoca Oct 07 '14 at 18:19
1

Has the `final` modifier anything to do with it though? Does classloader-reloading reinitialize the static variable if it's not `final`? – Yogu Oct 07 '14 at 19:29
@Yogu My understanding is that when the field is non-final, you can work around the problem by setting it to `null` when you clean up before undeployment. If the field is `final`, however, this workaround is no longer available. – Sergey Kalinichenko Oct 07 '14 at 19:37
Actually, the problem potentially caused by log frameworks is not related to the Logger variables being final or static. Setting the "log" attribute to null makes no difference. The problem is that the logging framework itself (such as the LoggerFactory) keeps a reference to the class for which the logger was created (MyClass), so that it cannot be garbage collected (assuming the logging framework is outside your application). Thus the solution is to tell the logging framework to let go of that class. This is described in my blog post linked above. – Mattias Jiderhamn Oct 22 '14 at 07:14

score 12 · Answer 2 · answered Oct 07 '14 at 17:51

The line of code is perfectly fine, and there is no real problem because the variable is final and static.

Maybe the person who made that comment was confused by the following.

In Java, when you create a public final static variable of type int (for example; it also works with some other types), then the compiler might, in places where you use that variable, substitute the actual constant value instead of a reference to the variable. For example, suppose you have the following:

class A {
    public final static int VALUE = 3;
}

public class B {
    public static void main(String[] args) {
        System.out.println(A.VALUE);
    }
}

When you compile and run this, it will obviously print 3.

Now suppose that you change class A and set VALUE = 4. You would expect that if you recompile class A and then run class B (without recompiling class B), you would see 4. But what happens is that you will still see 3. That is because the A.VALUE in class B was replaced by the actual constant value 3 when you compiled class B.

This is an optimization that the Java compiler does for constants.

As you can see this can cause problems, if you have such constants in the public API of your classes. Users of your code will have to recompile their code if you change the value of such constants.

But in the code you posted in your question this is not a problem, because the variable is private.

More details:

Java Language Specification 13.4.9

Declaring variable final and static

2 Answers2

Linked