[ARTEMIS-1636] Coverity: Null pointer dereferences (FORWARD_NULL) in JournalStorageManager.java

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Invalid
• Affects Version/s: 2.5.0
• Component/s: Broker
• External issue URL: https://scan7.coverity.com/reports.htm#v25199/p14213/fileInstanceId=46407695&defectInstanceId=9747152&mergedDefectId=1464350
• External issue ID: 1464350
• Assignee: Justin Bertram
• Reporter: Jiri Daněk
• Created: 24/Jan/18 17:14
• Updated: 11/Apr/24 03:29
• Resolved: 11/Apr/24 03:29

Description

There are multiple versions of the JournalStorageManager constructor.

The first overload is being called only from test code, as far as I can tell.

    public JournalStorageManager(final Configuration config,
                                final CriticalAnalyzer analyzer,
                                final ExecutorFactory executorFactory,
                                final ScheduledExecutorService scheduledExecutorService,
                                final ExecutorFactory ioExecutors) {
      this(config, analyzer, executorFactory, scheduledExecutorService, ioExecutors, null);
   }

This second overload is being called also from XmlDataExporter#process

   public JournalStorageManager(final Configuration config, CriticalAnalyzer analyzer, final ExecutorFactory executorFactory, final ExecutorFactory ioExecutors) {
      this(config, analyzer, executorFactory, null, ioExecutors, null);
   }

The problem with the second one is, according to Coverity

** CID 1464350:  Null pointer dereferences  (FORWARD_NULL)


________________________________________________________________________________________________________
*** CID 1464350:  Null pointer dereferences  (FORWARD_NULL)
/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java: 97 in org.apache.activemq.artemis.core.persistence.impl.journal.JournalStorageManager.<init>(org.apache.activemq.artemis.core.config.Configuration, org.apache.activemq.artemis.utils.critical.CriticalAnalyzer, org.apache.activemq.artemis.utils.ExecutorFactory, org.apache.activemq.artemis.utils.ExecutorFactory)()
91                                     final ScheduledExecutorService scheduledExecutorService,
92                                     final ExecutorFactory ioExecutors) {
93           this(config, analyzer, executorFactory, scheduledExecutorService, ioExecutors, null);
94        }
95
96        public JournalStorageManager(final Configuration config, CriticalAnalyzer analyzer, final ExecutorFactory executorFactory, final ExecutorFactory ioExecutors) {
>>>     CID 1464350:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing "null" to "JournalStorageManager", which dereferences it.
97           this(config, analyzer, executorFactory, null, ioExecutors, null);
98        }
99
100        public JournalStorageManager(final Configuration config,
101                                     final CriticalAnalyzer analyzer,
102                                     final ExecutorFactory executorFactory,

because in AbstractJournalStorageManager.java constructor, this happens

1. deref_parm_in_call: Function init dereferences criticalErrorListener. (The virtual call resolves to org.apache.activemq.artemis.core.persistence.impl.journal.JDBCJournalStorageManager.init.) [show details]
226      init(config, criticalErrorListener);

and in the #init method

59   protected synchronized void init(Configuration config, IOCriticalErrorListener criticalErrorListener) {
60      try {
61         final DatabaseStorageConfiguration dbConf = (DatabaseStorageConfiguration) config.getStoreConfiguration();
62         final JDBCJournalImpl bindingsJournal;
63         final JDBCJournalImpl messageJournal;
64         final JDBCSequentialFileFactory largeMessagesFactory;
  1. Condition dbConf.getDataSource() != null, taking true branch.
65         if (dbConf.getDataSource() != null) {
66            SQLProvider.Factory sqlProviderFactory = dbConf.getSqlProviderFactory();
  2. Condition sqlProviderFactory == null, taking true branch.
67            if (sqlProviderFactory == null) {
68               sqlProviderFactory = new PropertySQLProvider.Factory(dbConf.getDataSource());
69            }
70            bindingsJournal = new JDBCJournalImpl(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getBindingsTableName(), SQLProvider.DatabaseStoreType.BINDINGS_JOURNAL), dbConf.getBindingsTableName(), scheduledExecutorService, executorFactory.getExecutor(), criticalErrorListener);
71            messageJournal = new JDBCJournalImpl(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getMessageTableName(), SQLProvider.DatabaseStoreType.MESSAGE_JOURNAL), dbConf.getMessageTableName(), scheduledExecutorService, executorFactory.getExecutor(), criticalErrorListener);
  3. deref_parm_in_call: Function JDBCSequentialFileFactory dereferences criticalErrorListener. [show details]
72            largeMessagesFactory = new JDBCSequentialFileFactory(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getLargeMessageTableName(), SQLProvider.DatabaseStoreType.LARGE_MESSAGE), executorFactory.getExecutor(), criticalErrorListener);
73         } else {

Issue Links

• is part of: ARTEMIS-1400 FindBugs warnings
• relates to: ARTEMIS-1637 artemis data exp does not work with JDBC store