Bdtjtk

I came across some JMS calling code that initializes the JMS session inside of its constructor. The calling code implements the ExceptionListener interface and passes a reference to this to the connection factory object, as shown below:

public class JmsCode implements ExceptionListener {



    private static final Logger logger = LoggerFactory.getLogger(JmsCode.class);



    public JmsCode(String url, String username, String password, String trustStorePath, char trustStorePassword) throws JMSException {

        ActiveMQSslConnectionFactory connectionFactory = new ActiveMQSslConnectionFactory(url);

        connectionFactory.setUserName(username);

        connectionFactory.setPassword(password);

        connectionFactory.setTrustStore(trustStorePath);

        connectionFactory.setTrustStorePassword(new String(trustStorePassword));

        connectionFactory.setExceptionListener(this);



        Connection connection = connectionFactory.createConnection();

        connection.start();

        Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);

    }



    @Override

    public void onException(JMSException e) {

        logger.error("Unexpected JMS exception caught", e);

    }



}

I am wondering if it is safe to pass a reference to this from the JmsCode constructor given that the object hasn't been fully constructed yet. I came across a similar question which had me reading up on IBM's article on not publishing this during construction. While I agree with their reasoning, I am not sure if it applies in this case since the only thing the exception listener is doing is logging via a static and final member. Is the code above safe (ignoring someone else being tempted to change the exception listener method to use some instance state of the object)?

This is in fact unsafe publishing, and it's theoretically possible for another object to see this one in an inconsistent state.

That said, while this isn't a good pattern (and it's just shown here to demonstrate ExceptionListener), the logic of the constructor shows that the class is in fact fully constructed by the time that the this reference escapes (because it has nothing to construct), and so in this exact case there's nothing that can go wrong.

Whether it is safe or not depends on where that reference to this can escape to. If, as a consequence of this call, this could be read by another thread, then it is not safe.

If you want to ensure an instance is completely initialized for safe publication and you want to publish it, a constructor isn't the right place to publish it. Instead, you'll need to create a Factory object or a static factory method which can construct the object safely and then publish it before returning it to the caller.

It is completely safe. You are just passing the this reference, not using anything in the this scope.

Something will go wrong on that iff the .setExceptionListener(this) method performs something else than being a setter.