CODEWITHSUNDEEP
java
RVR
RVR

Reputation: 97

Sonar Violation: Dodgy - Write to static field from instance method

I have a variable - "protected static Context jndi;" in my class where "Context" is an interface . When i try to access it in the below mentioned method, it generates the sonar violation mentioned in the title

public JMSQueueResource createQueueResource(String queueBindingName, String qcfBindingName, boolean messagePersisted, boolean autoAcknowledge, boolean nonJMS) throws JMSException, NamingException {
    JMSQueueResource qResource = new JMSQueueResource();

    try {
        jndi = createInitialContext();
        if (queueConnectionFactory == null) {
            queueConnectionFactory = (QueueConnectionFactory) lookup(jndi, qcfBindingName);
        }
        qResource.theQueueConnection = queueConnectionFactory.createQueueConnection();

        if (autoAcknowledge) {
            qResource.theQueueSession = qResource.theQueueConnection.createQueueSession(false, Session.AUTO_ACKNOWLEDGE);
        }
        else {
            qResource.theQueueSession = qResource.theQueueConnection.createQueueSession(false, Session.CLIENT_ACKNOWLEDGE);
        }
        Queue queue = (Queue) lookup(jndi, queueBindingName);

        //if (nonJMS && queue instanceof com.ibm.mq.jms.MQQueue) {
        //    com.ibm.mq.jms.MQQueue q = (com.ibm.mq.jms.MQQueue) queue;
        //    q.setTargetClient(JMSC.MQJMS_CLIENT_NONJMS_MQ);
        //}

        qResource.theQueueSender = qResource.theQueueSession.createSender(queue);
        if (messagePersisted) {
            qResource.theQueueSender.setDeliveryMode(DeliveryMode.PERSISTENT);
        }
        else {
            qResource.theQueueSender.setDeliveryMode(DeliveryMode.NON_PERSISTENT);
        }
        qResource.theQueueConnection.start();
    } 
    catch (JMSException jmse) {
        throw jmse;
    } 
    catch (NamingException ne) {
        throw ne;
    } 
    finally {
        if(jndi != null){
            jndi.close();
        }
    }

    return qResource;
}

I could see there are suggestions like to use an Atomic Integer wrapper. What is the best fix for this problem?

Upvotes: 1

Views: 1939

Answers (1)

real_paul
real_paul

Reputation: 564

The sonar violation is a valid one as mutating a static variable from an instance method can lead to some pretty messed up behavior like:

  • How can you ensure that the field is initialized by an instance method before a static read access?
  • What happens when multiple threads access the field, directly or through the createQueueResource method?

Regarding the Java documentation, making it static and potentially accessed by multiple thread is a bad idea:

An InitialContext instance is not synchronized against concurrent access by multiple threads. Multiple threads each manipulating a different InitialContext instance need not synchronize. Threads that need to access a single InitialContext instance concurrently should synchronize amongst themselves and provide the necessary locking.

Having a local variable as suggested seems like a reasonable first way to avoid the warning and the related problems.

Whether the construction of the context is expensive depends also on the factory that is used to provide it.

First you need to worry about the correctness of the program, then you can optimize when you can test where the real bottlenecks are.

EDIT: This link should provide more insight into the Spring application context and how to leverage the dependency injection of the Spring container to make use of the Context instead of storing it in a variable in a class https://spring.io/understanding/application-context

Upvotes: 4

Related Questions