[DISCUSS] Coverity Scan for Artemis

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] Coverity Scan for Artemis

Jiri Danek
Hi folks, last week I submitted Artemis for inclusion into Coverity Scan.
Many Apache projects are there already, so you probably know about it. I
case you don't, it is a static code analysis as-a-service which is free of
charge for opensource.

The project is now approved by Coverity. You can open it and view the
analysis. You should be able to open https://scan.coverity.com/
projects/apache-activemq-artemis and click "Add me to project".

There are three visibility settings for the public. You can show either
nothing, show some basic stats like number of lines of code, number of
issues, and issues per kloc, or let anybody browse source and see
individual issues. I selected the second option.

There is a feature to sent e-mail with description of newly found issues to
a given e-mail address. I did not fill any address yet.

Currently, I am in "Admins" of the Coverity Scan project, with right to add
other users and submit project build for analysis*. This is something that
would need adjusting if you feel that Coverity Scan is useful and if
established people in the community step up wanting to take over (from me ;)

I uploaded all Artemis releases to date, ending with 2.0.0-snapshot from
last Friday, then looked at results and found that
1) It did not find the overflow I reported in
https://issues.apache.org/jira/browse/ARTEMIS-986; possible reason is that
this would be discovered by the FindBugs tool and in project settings it is
configured not to show FindBugs results (that is the default setting).
2) I tried to find some finds that are obviously not false positives, which
turned to be quite hard, but I got one
      - mismatched braces and indentation at https://scan7.coverity.com/
reports.htm#v25191/p14213/g25191g/fileInstanceId=9927190&defectInstanceId=
2438316&mergedDefectId=1409238&fileStart=251&fileEnd=500
(the whole if statement is weird, although since is in generated code, it
does not really count; I would probably still add the braces to the .jj
file, though)
3) Found obvious false positive where it is warning about a concurrent
modification of a concurrent hashset (because the hashset is implemented by
artemis and Coverity does not understand it is concurrent)
4) Most useful view of issues seems to be when I group issues by component
or filter out the "tests" and "test" components.

It would certainly require somebody who understands the code to review it
and decide if it is useful to have or not...

Cheers,

---------------------
* About submitting project build for analysis

The way I've been uploading the builds for analysis is by following their
quickstart ;) the only tricky part is the build command which I had to
struggle with a bit to disable errorprone

    /mnt/cov/cov-analysis-linux64-8.7.0/bin/cov-build --dir ../cov-int mvn
-Pexamples -DskipTests=true -Djavac-compiler-id=javac package

in addition, I had to go through pom.xml and artemis-selectors/pom.xml and
delete compiler args specific to errorprone; I did not find a way to do
this with maven options.

(cov-analysis-linux64-8.7.0 is just proprietary software downloaded from
Coverity that one needs to run)


--
Jiří Daněk,
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
Coverity sends e-mails with list of new issues it found for every submitted
build. Compared to last week, it now claims 4 new issues. None of them is
actually new. Two appear only now because last time I did not sent the C
code for analysis (in artemis-native) and the other two Java ones two were
there previously (maybe bug in Coverity). See for yourself below.

Regarding impact of the finds:

The mkstemp() thing is supposed to be an issue only with old glibc on Linux
and on AIX and HP-UX. I googled out that there is a good way to set umask()
for the process, because there is JVM running in it too... So probably
ignore.

Variable "iocb" going out of scope leaks the storage it points to. The
first time this is explained in a comment, the second instance of this does
not have a comment. I am not competent to judge, here.

Thread1 sets "state" to a new value. Now the two threads have an
inconsistent view of "state" and updates to fields correlated with "state"
may be lost. Regarding impact, with multithreading I just don't know.

Comparing "threadPool" to null implies that "threadPool" might be null. It
is necessary to look at Coverity website for this as the e-mail contains
only a snippet of the report available on Coverity web. Could it be
possible that  shutdownPool() is referring to wrong object in the log
messages it prints in case of exception? Looking at git history, I think
this is plausible explanation.

On Fri, Feb 24, 2017 at 11:35 PM, <[hidden email]> wrote:

Hi,

>
> Please find the latest report on new defect(s) introduced to Apache
> ActiveMQ Artemis found with Coverity Scan.
>
> 4 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
> Scan.
> 7 defect(s), reported by Coverity Scan earlier, were marked fixed in the
> recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 4 of 4 defect(s)
>
>
> ** CID 1411742:  Security best practices violations  (SECURE_TEMP)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 135 in
> JNI_OnLoad()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411742:  Security best practices violations  (SECURE_TEMP)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 135 in
> JNI_OnLoad()
> 129                 fprintf(stderr, "Could not allocate the 1 Mega Buffer
> for initializing files\n");
> 130                 return JNI_ERR;
> 131             }
> 132             memset(oneMegaBuffer, 0, ONE_MEGA);
> 133
> 134             sprintf (dumbPath, "%s/artemisJLHandler_XXXXXX", P_tmpdir);
> >>>     CID 1411742:  Security best practices violations  (SECURE_TEMP)
> >>>     Calling "mkstemp" without securely setting umask first.
> 135             dumbWriteHandler = mkstemp (dumbPath);
> 136
> 137             #ifdef DEBUG
> 138                fprintf (stdout, "Creating temp file %s for dumb
> writes\n", dumbPath);
> 139                fflush(stdout);
> 140             #endif
>
> ** CID 1411741:    (RESOURCE_LEAK)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 368 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 375 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411741:    (RESOURCE_LEAK)
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 368 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
> 362            if (iocb[i] == NULL) {
> 363                // it's unlikely this would happen at this point
> 364                // for that reason I'm not cleaning up individual IOCBs
> here
> 365                // we could increase support here with a cleanup of any
> previously allocated iocb
> 366                // But I'm afraid of adding not needed complexity here
> 367                throwOutOfMemoryError(env);
> >>>     CID 1411741:    (RESOURCE_LEAK)
> >>>     Variable "iocb" going out of scope leaks the storage it points to.
> 368                return NULL;
> 369            }
> 370         }
> 371
> 372         struct io_control * theControl = (struct io_control *)
> malloc(sizeof(struct io_control));
> 373         if (theControl == NULL) {
> /src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.c: 375 in
> Java_org_apache_activemq_artemis_jlibaio_LibaioContext_newContext()
> 369            }
> 370         }
> 371
> 372         struct io_control * theControl = (struct io_control *)
> malloc(sizeof(struct io_control));
> 373         if (theControl == NULL) {
> 374             throwOutOfMemoryError(env);
> >>>     CID 1411741:    (RESOURCE_LEAK)
> >>>     Variable "iocb" going out of scope leaks the storage it points to.
> 375             return NULL;
> 376         }
> 377
> 378         res = pthread_mutex_init(&(theControl->iocbLock), 0);
> 379         if (res) {
> 380             free(theControl);
>
> ** CID 1411740:    (LOCK_EVASION)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411740:    (LOCK_EVASION)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> 996           memoryManager = null;
> 997           backupManager = null;
> 998           storageManager = null;
> 999
> 1000           sessions.clear();
> 1001
> >>>     CID 1411740:    (LOCK_EVASION)
> >>>     Thread1 sets "state" to a new value. Now the two threads have an
> inconsistent view of "state" and updates to fields correlated with "state"
> may be lost.
> 1002           state = SERVER_STATE.STOPPED;
> 1003
> 1004           activationLatch.setCount(1);
> 1005
> 1006           // to display in the log message
> 1007           SimpleString tempNodeID = getNodeID();
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 1002 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> 996           memoryManager = null;
> 997           backupManager = null;
> 998           storageManager = null;
> 999
> 1000           sessions.clear();
> 1001
> >>>     CID 1411740:    (LOCK_EVASION)
> >>>     Thread1 sets "state" to a new value. Now the two threads have an
> inconsistent view of "state" and updates to fields correlated with "state"
> may be lost.
> 1002           state = SERVER_STATE.STOPPED;
> 1003
> 1004           activationLatch.setCount(1);
> 1005
> 1006           // to display in the log message
> 1007           SimpleString tempNodeID = getNodeID();
>
> ** CID 1411739:  Null pointer dereferences  (FORWARD_NULL)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 968 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1411739:  Null pointer dereferences  (FORWARD_NULL)
> /mnt/cov/activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
> 968 in
> org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.stop(boolean,
> boolean, boolean, boolean)()
> 962           stopComponent(memoryManager);
> 963
> 964           for (SecuritySettingPlugin securitySettingPlugin :
> configuration.getSecuritySettingPlugins()) {
> 965              securitySettingPlugin.stop();
> 966           }
> 967
> >>>     CID 1411739:  Null pointer dereferences  (FORWARD_NULL)
> >>>     Comparing "threadPool" to null implies that "threadPool" might be
> null.
> 968           if (threadPool != null && !threadPoolSupplied) {
> 969              shutdownPool(threadPool);
> 970           }
> 971
> 972           if (ioExecutorPool != null) {
> 973              shutdownPool(ioExecutorPool);
>
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit,
> https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wAHEVPKzmIIOtLlZCpkesdZoOgp56XudeOz1563G73aJe0vuBiY0pxBectz6EdXD0arBJ9IkKlOXWgY2pmVMD4ceyWTtAI2YEWTfDs9EkUt0Cf6E3xISQPQw00kFJBI-2Bfnz1EJpNJbmpbiGiAtWTtMARP9FhjmFQh3KNyIhAPOiCg-3D-3D
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
Hello, let me sent one more e-mail in this thread ;P Trying to evaluate
what Coverity found, I opened some Jiras where I understood the problem and
it seemed real, and marked the Coverity find as ignored if .

The Jiras are

    ARTEMIS-996 Simplify and deduplicate lookupHome(path) in
artemis-maven-plugin
    ARTEMIS-993 ClientConsumerImpl.java contains unreachable code
    ARTEMIS-991 Null dereference after hitting Ctrl+d when prompted for
password in `artemis create`

There I one thing that I know about but not reported, about if you put
"_AMQ_NULL" as value for a float property in a message to the xml used in
`artemis data imp`, you get an exception out of the import command.

At this point, I think I am through all I can judge, but still a lot of
finds still remains to be acted upon. For example, I did not investigate
anything very hard; if it was not obvious or seemed easy to figure out, I
skipped it. I also skipped over all multithreading finds as a matter of
principle, because I don't have experience with multithreaded Java. If
anybody wants to look at it more, please go ahead.

Few random examples of the multithreading finds

86         if (tx != null) {

CID 1409940 (#1 of 1): Thread deadlock (LOCK_INVERSION)4. lock_order: Acquiring
lock TransactionImpl.timeoutLock while holding DuplicateIDCacheImpl.this
conflicts with the lock order established elsewhere. (The virtual call
resolves to
org.apache.activemq.artemis.core.transaction.impl.TransactionImpl.markAsRollbackOnly
.) [show details
<https://scan7.coverity.com/eventId=2434490-3&modelId=2434490-0&fileInstanceId=9909080&filePath=%2Fartemis-server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Factivemq%2Fartemis%2Fcore%2Ftransaction%2Fimpl%2FTransactionImpl.java&fileStart=459&fileEnd=477>
]
187            tx.markAsRollbackOnly(new ActiveMQDuplicateIdException());

----------------------------------------------------------------------------------------------------------------------------

261      finally {

CID 1409057: Unguarded write (GUARDED_BY_VIOLATION) [select issue
<https://scan7.coverity.com/defectInstanceId=2430696&fileInstanceId=9893725&mergedDefectId=1409057>
]

5. thread2_modifies_field: Thread2 sets dispatching to a new value. Note
that this write can be reordered at runtime to occur before instructions
that do not access this field, even into (but not to the other side of)
preceding locked regions. Control is switched back to Thread1.

CID 1409135 (#1 of 1): Check of thread-shared field evades lock acquisition
(LOCK_EVASION)6. thread1_overwrites_value_in_field: Thread1 sets dispatching
to a new value. Now the two threads have an inconsistent view of dispatching
and updates to fields correlated with dispatching may be lost.

Guard the modification of dispatching and the read used to decide whether
to modify dispatching with the same set of locks.
262         dispatching = false;

----------------------------------------------------------------------------------------------------------------------------

148   public int getPercentageBlocked() {

CID 1409537 (#1 of 1): Result is not floating-point
(UNINTENDED_INTEGER_DIVISION)integer_division: Dividing integer expressions
flowControlInfo.getSendsBlocked() and flowControlInfo.getTotalSends(), and
then converting the integer quotient to type double. Any remainder, or
fractional part of the quotient, is ignored.

To compute and use a non-integer quotient, change or cast either operand to
type double. If integer division is intended, consider indicating that by
casting the result to type long .
149      double value = flowControlInfo.getSendsBlocked() / flowControlInfo.
getTotalSends();
150      return (int) value * 100;
151   }

----------------------------------------------------------------------------------------------------------------------------

 68   public void start() throws SQLException {

1. assign_to_field: The expression connect() assigns a new value to this
.connection, a field whose contents are used as a lock. The locking
behavior of this function may allow this assignment to occur multiple times.
 [show details
<https://scan7.coverity.com/eventId=2509558-0&modelId=2509558-0&fileInstanceId=10277616&filePath=%2Fmnt%2Fcov%2Factivemq-artemis%2Fartemis-jdbc-store%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Factivemq%2Fartemis%2Fjdbc%2Fstore%2Fdrivers%2FAbstractJDBCDriver.java&fileStart=102&fileEnd=131>
]
 69      connect();

2. lock_acquire: Acquiring lock AbstractJDBCDriver.connection.

CID 1409988 (#1 of 1): Bad choice of lock object (BAD_LOCK_OBJECT)3.
lock_on_assigned_field: Locking on the object referenced by field connection.
This lock acquisition may race with another thread assigning to this field.
The contents of connection may change while a thread is inside the critical
section, allowing two threads to enter the critical section simultaneously.

Instead of using connection as a lock, create a final field of type Object
which is only used as a lock.
 70      synchronized (connection) {
 71         createSchema();
 72         prepareStatements();
 73      }
 74   }

Cheers,
--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

nigro_franz
Hi Jiri!
It is awesome...the issue on the AbstractJDBCDriver seems correct, considering that the analysis is not aware of the lifecycle of the connection field, as intended by the developer!

Cheers,
Francesco
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
In reply to this post by Jiri Danek
(two corrections, I noticed I did not wrote what I meant, previously)

On Sat, Feb 25, 2017 at 8:47 AM, Jiri Danek <[hidden email]> wrote:

> The mkstemp() thing is supposed to be an issue only with old glibc on
> Linux and on AIX and HP-UX. I googled out that there is a good way to set
> umask() for the process, because there is JVM running in it too... So
> probably ignore.


I meant to say "there is _not_ a good way"

On Sat, Feb 25, 2017 at 9:11 AM, Jiri Danek <[hidden email]> wrote:

> Hello, let me sent one more e-mail in this thread ;P Trying to evaluate
> what Coverity found, I opened some Jiras where I understood the problem and
> it seemed real, and marked the Coverity find as ignored if .


I meant to add "marked as ignored if I was reasonably sure that it is
intended". You can filter for this on the Coverity web. There were two
issues where it was obvious, and one where I think it is not worth fixing:

204                     if (futureListener != null) {
205
// TODO BEFORE MERGE: (is null a good option here?)

CID 1409858 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
var_deref_model: Passing null to operationComplete, which dereferences it.
(The virtual call resolves to io.netty.channel.embedded.EmbeddedChannel.1
.operationComplete.)
206                        futureListener.operationComplete(null);
207                     }

It fails the substitution principle (most of ChannelFutureListener
implementations that exist in Netty would fail with NPE there), but what I
found by searching in Intellij is that only null or a barebone
ChannelFutureListener that does work are ever passed there. To fix it, one
would have to implement proper instance of ChannelFuture to pass as
argument instead of that null, which would be 100 lines of essentially
fluff, and there would still be problem because the future needs to be able
to return non-null instance of Channel, of which there isn't any. I simply
cannot think of reasonable fix. And it seems to be working all right. Maybe
change the comment ;)
--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

clebertsuconic
Where is this "// TODO BEFORE MERGE: (is null a good option here?)"


This some mark I usually do, I meant to fix it before I sent the commit :)

On Sat, Feb 25, 2017 at 5:27 AM, Jiri Danek <[hidden email]> wrote:

> (two corrections, I noticed I did not wrote what I meant, previously)
>
> On Sat, Feb 25, 2017 at 8:47 AM, Jiri Danek <[hidden email]> wrote:
>
>> The mkstemp() thing is supposed to be an issue only with old glibc on
>> Linux and on AIX and HP-UX. I googled out that there is a good way to set
>> umask() for the process, because there is JVM running in it too... So
>> probably ignore.
>
>
> I meant to say "there is _not_ a good way"
>
> On Sat, Feb 25, 2017 at 9:11 AM, Jiri Danek <[hidden email]> wrote:
>
>> Hello, let me sent one more e-mail in this thread ;P Trying to evaluate
>> what Coverity found, I opened some Jiras where I understood the problem and
>> it seemed real, and marked the Coverity find as ignored if .
>
>
> I meant to add "marked as ignored if I was reasonably sure that it is
> intended". You can filter for this on the Coverity web. There were two
> issues where it was obvious, and one where I think it is not worth fixing:
>
> 204                     if (futureListener != null) {
> 205
> // TODO BEFORE MERGE: (is null a good option here?)
>
> CID 1409858 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
> var_deref_model: Passing null to operationComplete, which dereferences it.
> (The virtual call resolves to io.netty.channel.embedded.EmbeddedChannel.1
> .operationComplete.)
> 206                        futureListener.operationComplete(null);
> 207                     }
>
> It fails the substitution principle (most of ChannelFutureListener
> implementations that exist in Netty would fail with NPE there), but what I
> found by searching in Intellij is that only null or a barebone
> ChannelFutureListener that does work are ever passed there. To fix it, one
> would have to implement proper instance of ChannelFuture to pass as
> argument instead of that null, which would be 100 lines of essentially
> fluff, and there would still be problem because the future needs to be able
> to return non-null instance of Channel, of which there isn't any. I simply
> cannot think of reasonable fix. And it seems to be working all right. Maybe
> change the comment ;)
> --
> Jiří Daněk



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
I put a new snapshot into coverity yesterday. There was nothing new found
and nothing old fixed. The table with component breakdown on the Coverity
website is still broken, not sure why.

    Your request for analysis of Apache ActiveMQ Artemis has been completed
successfully.
    The results are available at
https://scan.coverity.com/projects/apache-activemq-artemis
    Analysis Summary:
       New defects found: 0
       Defects eliminated: 0

There is 753 outstanding finds now, more than half of it in tests and
examples, quite lot of it is caused by closing two JMS sessions in a
finally block, where closing the first may throw exception, which would
mean the second one does not get closed. Something that is not wroth
fixing, I'd say.

On Sat, Feb 25, 2017 at 9:23 AM, nigro_franz <[hidden email]> wrote:

> It is awesome...the issue on the AbstractJDBCDriver seems correct,
> considering that the analysis is not aware of the lifecycle of the
> connection field, as intended by the developer!
>

It can probably be made aware of things like this [1], but it requires
adding hints to the sourcecode. I did it only once long time ago in C++, so
I don't recall the details. I have a feeling it is not worth it for Artemis.

[1] https://scan.coverity.com/models#java_models_annotations ; accessing
the link requires registration ;(

On Sat, Feb 25, 2017 at 4:18 PM, Clebert Suconic <[hidden email]>
wrote:

> Where is this "// TODO BEFORE MERGE: (is null a good option here?)"
>
>
> This some mark I usually do, I meant to fix it before I sent the commit :)
>

in artemis-server/src/main/java/org/apache/activemq/artemis/
core/remoting/impl/invm/InVMConnection.java
<https://github.com/apache/activemq-artemis/blob/fbc77b44c2bf72cede87b7dd3b74ba2604ccb7b6/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMConnection.java>,

--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
Hi, this is Coverity Scan for current git master, after the message
encoding PR was merged. Forwarded with hope something on it is real issue.
See https://scan.coverity.com/projects/apache-activemq-artemis for full
details.

---------- Forwarded message ----------
From: <[hidden email]>
Date: Mon, Mar 6, 2017 at 4:45 PM
Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
To: [hidden email]



Hi,

Please find the latest report on new defect(s) introduced to Apache
ActiveMQ Artemis found with Coverity Scan.

16 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
Scan.
30 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 16 of 16 defect(s)


** CID 1415199:  Null pointer dereferences  (REVERSE_INULL)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
2491 in
org.apache.activemq.artemis.core.server.impl.QueueImpl.makeCopy(org.apache.activemq.artemis.core.server.MessageReference,
boolean, boolean)()


________________________________________________________________________________________________________
*** CID 1415199:  Null pointer dereferences  (REVERSE_INULL)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
2491 in
org.apache.activemq.artemis.core.server.impl.QueueImpl.makeCopy(org.apache.activemq.artemis.core.server.MessageReference,
boolean, boolean)()
2485
2486           long newID = storageManager.generateID();
2487
2488           Message copy = message.copy(newID);
2489
2490           if (copyOriginalHeaders) {
>>>     CID 1415199:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "ref" suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
2491              copy.referenceOriginalMessage(message, ref != null ?
ref.getQueue().getName().toString() : null);
2492           }
2493
2494           if (expiry) {
2495
copy.putLongProperty(Message.HDR_ACTUAL_EXPIRY_TIME.toString(),
System.currentTimeMillis());
2496           }

** CID 1415198:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTRetainMessageManager.java:
58 in
org.apache.activemq.artemis.core.protocol.mqtt.MQTTRetainMessageManager.handleRetainedMessage(org.apache.activemq.artemis.api.core.Message,
java.lang.String, boolean,
org.apache.activemq.artemis.core.transaction.Transaction)()


________________________________________________________________________________________________________
*** CID 1415198:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTRetainMessageManager.java:
58 in
org.apache.activemq.artemis.core.protocol.mqtt.MQTTRetainMessageManager.handleRetainedMessage(org.apache.activemq.artemis.api.core.Message,
java.lang.String, boolean,
org.apache.activemq.artemis.core.transaction.Transaction)()
52              queue =
session.getServerSession().createQueue(retainAddress, retainAddress, null,
false, true);
53           }
54
55
56           try (LinkedListIterator<MessageReference> iterator =
queue.iterator()) {
57              synchronized (queue) {
>>>     CID 1415198:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "iterator".
58                 if (iterator.hasNext()) {
59                    MessageReference ref = iterator.next();
60                    iterator.remove();
61                    queue.acknowledge(tx, ref);
62                 }
63

** CID 1415197:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java:
238 in
org.apache.activemq.artemis.core.protocol.openwire.amq.AMQSession.sendMessage(org.apache.activemq.artemis.core.server.MessageReference,
org.apache.activemq.artemis.api.core.Message,
org.apache.activemq.artemis.core.server.ServerConsumer, int)()


________________________________________________________________________________________________________
*** CID 1415197:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java:
238 in
org.apache.activemq.artemis.core.protocol.openwire.amq.AMQSession.sendMessage(org.apache.activemq.artemis.core.server.MessageReference,
org.apache.activemq.artemis.api.core.Message,
org.apache.activemq.artemis.core.server.ServerConsumer, int)()
232        public int sendMessage(MessageReference reference,
233
org.apache.activemq.artemis.api.core.Message message,
234                               ServerConsumer consumer,
235                               int deliveryCount) {
236           AMQConsumer theConsumer = (AMQConsumer)
consumer.getProtocolData();
237           // TODO: use encoders and proper conversions here
>>>     CID 1415197:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "theConsumer".
238           return theConsumer.handleDeliver(reference, message.toCore(),
deliveryCount);
239        }
240
241        @Override
242        public int sendLargeMessage(MessageReference reference,
243
org.apache.activemq.artemis.api.core.Message message,

** CID 1415196:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-stomp-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/StompSession.java:
395 in
org.apache.activemq.artemis.core.protocol.stomp.StompSession.sendInternalLarge(org.apache.activemq.artemis.core.message.impl.CoreMessage,
boolean)()


________________________________________________________________________________________________________
*** CID 1415196:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-stomp-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/StompSession.java:
395 in
org.apache.activemq.artemis.core.protocol.stomp.StompSession.sendInternalLarge(org.apache.activemq.artemis.core.message.impl.CoreMessage,
boolean)()
389           long id = storageManager.generateID();
390           LargeServerMessage largeMessage =
storageManager.createLargeMessage(id, message);
391
392           byte[] bytes = new byte[message.getBodyBuffer().writerIndex()
- CoreMessage.BODY_OFFSET];
393           message.getBodyBuffer().readBytes(bytes);
394
>>>     CID 1415196:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "largeMessage".
395           largeMessage.addBytes(bytes);
396
397           largeMessage.releaseResources();
398
399           largeMessage.putLongProperty(Message.HDR_LARGE_BODY_SIZE,
bytes.length);
400

** CID 1415195:  Null pointer dereferences  (NULL_RETURNS)
/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java:
549 in org.apache.activemq.artemis.api.core.Message.toMap()()


________________________________________________________________________________________________________
*** CID 1415195:  Null pointer dereferences  (NULL_RETURNS)
/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java:
549 in org.apache.activemq.artemis.api.core.Message.toMap()()
543
544        /**
545         * @return Returns the message in Map form, useful when encoding
to JSON
546         */
547        default Map<String, Object> toMap() {
548           Map map = toPropertyMap();
>>>     CID 1415195:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "map".
549           map.put("messageID", getMessageID());
550           Object userID = getUserID();
551           if (getUserID() != null) {
552              map.put("userID", "ID:" + userID.toString());
553           }
554

** CID 1415194:  Null pointer dereferences  (NULL_RETURNS)
/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java:
173 in
org.apache.activemq.artemis.api.core.Message.getDeliveryAnnotationPropertyString(org.apache.activemq.artemis.api.core.SimpleString)()


________________________________________________________________________________________________________
*** CID 1415194:  Null pointer dereferences  (NULL_RETURNS)
/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java:
173 in
org.apache.activemq.artemis.api.core.Message.getDeliveryAnnotationPropertyString(org.apache.activemq.artemis.api.core.SimpleString)()
167
168        default SimpleString
getDeliveryAnnotationPropertyString(SimpleString property) {
169           Object obj = getDeliveryAnnotationProperty(property);
170           if (obj instanceof SimpleString) {
171              return (SimpleString)obj;
172           } else {
>>>     CID 1415194:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "obj".
173              return SimpleString.toSimpleString(obj.toString());
174           }
175        }
176
177        default void cleanupInternalProperties() {
178           // only on core

** CID 1415193:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java:
489 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPSessionCallback.sendMessage(org.apache.activemq.artemis.core.server.MessageReference,
org.apache.activemq.artemis.api.core.Message,
org.apache.activemq.artemis.core.server.ServerConsumer, int)()


________________________________________________________________________________________________________
*** CID 1415193:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java:
489 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPSessionCallback.sendMessage(org.apache.activemq.artemis.core.server.MessageReference,
org.apache.activemq.artemis.api.core.Message,
org.apache.activemq.artemis.core.server.ServerConsumer, int)()
483
484
message.removeProperty(ActiveMQConnection.CONNECTION_ID_PROPERTY_NAME.toString());
485
486           ProtonServerSenderContext plugSender =
(ProtonServerSenderContext) consumer.getProtocolContext();
487
488           try {
>>>     CID 1415193:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "plugSender".
489              return
plugSender.deliverMessage(CoreAmqpConverter.checkAMQP(message),
deliveryCount);
490           } catch (Exception e) {
491              synchronized (connection.getLock()) {
492                 plugSender.getSender().setCondition(new
ErrorCondition(AmqpError.INTERNAL_ERROR, e.getMessage()));
493                 connection.flush();
494              }

** CID 1415192:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java:
1040 in
org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl$3.run()()


________________________________________________________________________________________________________
*** CID 1415192:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java:
1040 in
org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl$3.run()()
1034        // Inner classes
1035        //
------------------------------------------------------------------------
1036
1037        private final Runnable resumeLargeMessageRunnable = new
Runnable() {
1038           @Override
1039           public void run() {
>>>     CID 1415192:  Program hangs  (LOCK_INVERSION)
>>>     Acquiring lock "ServerConsumerImpl.lock".
1040              synchronized (lock) {
1041                 try {
1042                    if (largeMessageDeliverer == null ||
largeMessageDeliverer.deliver()) {
1043                       forceDelivery();
1044                    }
1045                 } catch (Exception e) {

** CID 1415191:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
924 in
org.apache.activemq.artemis.core.server.impl.QueueImpl.hasMatchingConsumer(org.apache.activemq.artemis.api.core.Message)()


________________________________________________________________________________________________________
*** CID 1415191:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
924 in
org.apache.activemq.artemis.core.server.impl.QueueImpl.hasMatchingConsumer(org.apache.activemq.artemis.api.core.Message)()
918        public synchronized Set<Consumer> getConsumers() {
919           return new HashSet<>(consumerSet);
920        }
921
922        @Override
923        public boolean hasMatchingConsumer(final Message message) {
>>>     CID 1415191:  Concurrent data access violations
(GUARDED_BY_VIOLATION)
>>>     Accessing "consumerList" without holding lock "QueueImpl.this".
Elsewhere,
"org.apache.activemq.artemis.core.server.impl.QueueImpl.consumerList" is
accessed with "QueueImpl.this" held 13 out of 17 times.
924           for (ConsumerHolder holder : consumerList) {
925              Consumer consumer = holder.consumer;
926
927              if (consumer instanceof Redistributor) {
928                 continue;
929              }

** CID 1415190:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java:
164 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.getMessageAnnotations()()


________________________________________________________________________________________________________
*** CID 1415190:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java:
164 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.getMessageAnnotations()()
158              parsedHeaders = true;
159           }
160        }
161
162        public MessageAnnotations getMessageAnnotations() {
163           parseHeaders();
>>>     CID 1415190:  Concurrent data access violations
(GUARDED_BY_VIOLATION)
>>>     Accessing "_messageAnnotations" without holding lock
"AMQPMessage.this". Elsewhere,
"org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage._messageAnnotations"
is accessed with "AMQPMessage.this" held 2 out of 3 times.
164           return _messageAnnotations;
165        }
166
167        public Header getHeader() {
168           parseHeaders();
169           return _header;

** CID 1415189:  Null pointer dereferences  (FORWARD_NULL)
/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/ProtonTest.java:
1296 in
org.apache.activemq.artemis.tests.integration.amqp.ProtonTest$7.run()()


________________________________________________________________________________________________________
*** CID 1415189:  Null pointer dereferences  (FORWARD_NULL)
/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/ProtonTest.java:
1296 in
org.apache.activemq.artemis.tests.integration.amqp.ProtonTest$7.run()()
1290                    exceptions.add(e);
1291                    e.printStackTrace();
1292                 } finally {
1293                    try {
1294                       // if the createconnecion wasn't commented out
1295                       if (connectionConsumer != connection) {
>>>     CID 1415189:  Null pointer dereferences  (FORWARD_NULL)
>>>     Calling a method on null object "connectionConsumer".
1296                          connectionConsumer.close();
1297                       }
1298                    } catch (Throwable ignored) {
1299                       // NO OP
1300                    }
1301                 }

** CID 1415188:  Null pointer dereferences  (FORWARD_NULL)


________________________________________________________________________________________________________
*** CID 1415188:  Null pointer dereferences  (FORWARD_NULL)
/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataImporter.java:
464 in
org.apache.activemq.artemis.cli.commands.tools.XmlDataImporter.processMessageProperties(org.apache.activemq.artemis.api.core.Message)()
458                 message.putIntProperty(key, Integer.parseInt(value));
459                 break;
460              case XmlDataConstants.PROPERTY_TYPE_LONG:
461                 message.putLongProperty(key, Long.parseLong(value));
462                 break;
463              case XmlDataConstants.PROPERTY_TYPE_SIMPLE_STRING:
>>>     CID 1415188:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing "null" to "putStringProperty", which dereferences it. (The
virtual call resolves to
"org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.putStringProperty".)
464                 message.putStringProperty(new SimpleString(key), value
== null ? null : SimpleString.toSimpleString(value));
465                 break;
466              case XmlDataConstants.PROPERTY_TYPE_STRING:
467                 message.putStringProperty(key, value);
468                 break;
469           }

** CID 1415187:  Null pointer dereferences  (FORWARD_NULL)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
2377 in
org.apache.activemq.artemis.core.server.impl.QueueImpl.moveBetweenSnFQueues(org.apache.activemq.artemis.api.core.SimpleString,
org.apache.activemq.artemis.core.transaction.Transaction,
org.apache.activemq.artemis.core.server.MessageReference)()


________________________________________________________________________________________________________
*** CID 1415187:  Null pointer dereferences  (FORWARD_NULL)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
2377 in
org.apache.activemq.artemis.core.server.impl.QueueImpl.moveBetweenSnFQueues(org.apache.activemq.artemis.api.core.SimpleString,
org.apache.activemq.artemis.core.transaction.Transaction,
org.apache.activemq.artemis.core.server.MessageReference)()
2371
2372                 // there should only be one of these properties so
potentially save some loop iterations
2373                 break;
2374              }
2375           }
2376
>>>     CID 1415187:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "oldRouteToIDs" to "wrap", which dereferences
it.
2377           ByteBuffer oldBuffer = ByteBuffer.wrap(oldRouteToIDs);
2378
2379           RoutingContext routingContext = new RoutingContextImpl(tx);
2380
2381           /* this algorithm will look at the old route and find the
new remote queue bindings where the messages should go
2382            * and route them there directly

** CID 1415186:  Null pointer dereferences  (FORWARD_NULL)


________________________________________________________________________________________________________
*** CID 1415186:  Null pointer dereferences  (FORWARD_NULL)
/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/jms/ServerJMSMessage.java:
168 in
org.apache.activemq.artemis.protocol.amqp.converter.jms.ServerJMSMessage.setJMSDestination(javax.jms.Destination)()
162           }
163        }
164
165        @Override
166        public final void setJMSDestination(Destination destination)
throws JMSException {
167           if (destination == null) {
>>>     CID 1415186:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing "null" to "setAddress", which dereferences it. (The virtual
call resolves to
"org.apache.activemq.artemis.core.message.impl.CoreMessage.setAddress".)
168              message.setAddress((SimpleString)null);
169           } else {
170              message.setAddress(((ActiveMQDestination)
destination).getSimpleAddress());
171           }
172
173        }

** CID 1409771:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java:
136 in
org.apache.activemq.artemis.core.paging.cursor.impl.PageCursorProviderImpl.getPageCache(long)()


________________________________________________________________________________________________________
*** CID 1409771:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java:
136 in
org.apache.activemq.artemis.core.paging.cursor.impl.PageCursorProviderImpl.getPageCache(long)()
130        }
131
132        @Override
133        public PageCache getPageCache(final long pageId) {
134           try {
135              PageCache cache;
>>>     CID 1409771:  Program hangs  (LOCK_INVERSION)
>>>     Acquiring lock "PageCursorProviderImpl.softCache".
136              synchronized (softCache) {
137                 if (pageId > pagingStore.getCurrentWritingPage()) {
138                    return null;
139                 }
140
141                 cache = softCache.get(pageId);

** CID 1409143:    (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java:
1178 in
org.apache.activemq.artemis.core.paging.cursor.impl.PageSubscriptionImpl$CursorIterator.moveNext()()
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java:
1178 in
org.apache.activemq.artemis.core.paging.cursor.impl.PageSubscriptionImpl$CursorIterator.moveNext()()


________________________________________________________________________________________________________
*** CID 1409143:    (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java:
1178 in
org.apache.activemq.artemis.core.paging.cursor.impl.PageSubscriptionImpl$CursorIterator.moveNext()()
1172                 PagedReference message;
1173
1174                 PagePosition lastPosition = position;
1175                 PagePosition tmpPosition = position;
1176
1177                 do {
>>>     CID 1409143:    (LOCK_INVERSION)
>>>     Acquiring lock "CursorIterator.redeliveries".
1178                    synchronized (redeliveries) {
1179                       PagePosition redelivery = redeliveries.poll();
1180
1181                       if (redelivery != null) {
1182                          // There's a redelivery pending, we will get
it out of that pool instead
1183                          isredelivery = true;
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java:
1178 in
org.apache.activemq.artemis.core.paging.cursor.impl.PageSubscriptionImpl$CursorIterator.moveNext()()
1172                 PagedReference message;
1173
1174                 PagePosition lastPosition = position;
1175                 PagePosition tmpPosition = position;
1176
1177                 do {
>>>     CID 1409143:    (LOCK_INVERSION)
>>>     Acquiring lock "CursorIterator.redeliveries".
1178                    synchronized (redeliveries) {
1179                       PagePosition redelivery = redeliveries.poll();
1180
1181                       if (redelivery != null) {
1182                          // There's a redelivery pending, we will get
it out of that pool instead
1183                          isredelivery = true;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit,
https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wDE9Yd4-2FCN-2Fy8UFapwlOiQj8BC676trfKvMtmIKbTBk8rHbpYpVtj7IPdke9oXdMki3q5c7-2BVqCO-2BSILe5W0BIOA4gulo2mvcwplLzlXigXR7XrojLNrlz2bHWZW5sVS4IJKi-2BQVe-2FN1h8IMGZo9AvK3QbokcUD3GWPImCN6KyNUg-3D-3D

To manage Coverity Scan email notifications for "[hidden email]", click
https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRbVDbis712qZDP-2FA8y06Nq4ayNyW5oN6bOTmA3eqVv9aekj-2F0CBXY1ycW0hZjdXilegPumDmuHGy9LK5TWnfvYxQ6gVG5mrC9yxgLSOeowoGl-2Bn30PFzC603ZASlb9JLM4-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wDE9Yd4-2FCN-2Fy8UFapwlOiQj9DmySmy47ocPcPljvITSQpPIWDxbnsrnZ0oVCst2rpreOJrYQyXO-2BxqlNe-2B2h3bTzQrOWh7fZ2rPBoleEXW5Md4T5gnGxY9t1dCRBoh5bwZqGd-2BeQrfKhmuY1FjjqOGrc12hwsXyA5VnXCb0EwiKlw-3D-3D

--
Jiří Daněk
Messaging QA
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
---------- Forwarded message ----------
From: <[hidden email]>
Date: Fri, Mar 17, 2017 at 10:59 AM
Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
To: [hidden email]



Hi,

Please find the latest report on new defect(s) introduced to Apache
ActiveMQ Artemis found with Coverity Scan.

4 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
Scan.
5 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)


** CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
/tests/integration-tests/src/test/java/org/apache/activemq/a
rtemis/tests/integration/client/ConsumerTest.java: 419 in
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
int)()


____________________________________________________________
____________________________________________
*** CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
/tests/integration-tests/src/test/java/org/apache/activemq/a
rtemis/tests/integration/client/ConsumerTest.java: 419 in
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
int)()
413
414              TextMessage msg = (TextMessage) consumer.receive(1000);
415              Assert.assertEquals("testSelectorExampleFromSpecs:2",
msg.getText());
416
417           } finally {
418              connection.close();
>>>     CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
>>>     Variable "factorySend" going out of scope leaks the resource it
refers to.
419           }
420        }
421
422        @Test
423        public void testConsumerAckImmediateAutoCommitTrue() throws
Exception {
424           ClientSessionFactory sf = createSessionFactory(locator);

** CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
/tests/integration-tests/src/test/java/org/apache/activemq/a
rtemis/tests/integration/client/ConsumerTest.java: 317 in
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
int)()


____________________________________________________________
____________________________________________
*** CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
/tests/integration-tests/src/test/java/org/apache/activemq/a
rtemis/tests/integration/client/ConsumerTest.java: 317 in
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
int)()
311        public void internalSend(int protocolSender, int
protocolConsumer) throws Throwable {
312
313           ConnectionFactory factorySend = createFactory(protocolSender);
314           ConnectionFactory factoryConsume = protocolConsumer ==
protocolSender ? factorySend : createFactory(protocolConsumer);
315
316
>>>     CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "factorySend".
317           Connection connection = factorySend.createConnection();
318
319           try {
320              Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
321              javax.jms.Queue queue = session.createQueue(QUEUE.toSt
ring());
322              MessageProducer producer = session.createProducer(queue);

** CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
/artemis-server/src/main/java/org/apache/activemq/artemis/co
re/paging/cursor/PagedReferenceImpl.java: 137 in
org.apache.activemq.artemis.core.paging.cursor.PagedReferenc
eImpl.getScheduledDeliveryTime()()


____________________________________________________________
____________________________________________
*** CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
/artemis-server/src/main/java/org/apache/activemq/artemis/co
re/paging/cursor/PagedReferenceImpl.java: 137 in
org.apache.activemq.artemis.core.paging.cursor.PagedReferenc
eImpl.getScheduledDeliveryTime()()
131
132        @Override
133        public long getScheduledDeliveryTime() {
134           if (deliveryTime == null) {
135              try {
136                 Message msg = getMessage();
>>>     CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
>>>     Unboxing null object "msg.getScheduledDeliveryTime()".
137                 return msg.getScheduledDeliveryTime();
138              } catch (Throwable e) {
139                 ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
140                 return 0L;
141              }
142           }

** CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
2373 in org.apache.activemq.artemis.core.server.impl.QueueImpl.moveB
etweenSnFQueues(org.apache.activemq.artemis.api.core.SimpleString,
org.apache.activemq.artemis.core.transaction.Transaction,
org.apache.activemq.artemis.core.server.MessageReference)()


____________________________________________________________
____________________________________________
*** CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
2373 in org.apache.activemq.artemis.core.server.impl.QueueImpl.moveB
etweenSnFQueues(org.apache.activemq.artemis.api.core.SimpleString,
org.apache.activemq.artemis.core.transaction.Transaction,
org.apache.activemq.artemis.core.server.MessageReference)()
2367           Binding targetBinding;
2368
2369           // remove the old route
2370           for (SimpleString propName : copyMessage.getPropertyNames())
{
2371              if (propName.startsWith(Message.HDR_ROUTE_TO_IDS)) {
2372                 oldRouteToIDs = (byte[]) copyMessage.removeProperty(pro
pName.toString());
>>>     CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "oldRouteToIDs".
2373                 final String hashcodeToString =
oldRouteToIDs.toString(); // don't use Arrays.toString(..) here
2374                 logger.debug("Removed property from message: " +
propName + " = " + hashcodeToString + " (" +
ByteBuffer.wrap(oldRouteToIDs).getLong()
+ ")");
2375
2376                 // there should only be one of these properties so
potentially save some loop iterations
2377                 break;
2378              }


____________________________________________________________
____________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.n
et/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2
MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8P
Ekb70GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvz
M9wDEBs83dhsMiiHLs6eTC5vNN8SQrPHx5Y-2FunG8Ul9-2FtGelYwdL08hg
4PK4L5N-2FrzHT6HG-2B7H7X8f0pdTqUNCmfC4-2BO8FqmU2U7GA8fkpPkZD
935bABKyBgetbGXKZruAnUZrfa21ulwAIB5-2FFiJfax8q9bLx0-2B99BNXR
eKbArA-2BJg-3D-3D
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
---------- Forwarded message ----------
From: <[hidden email]>
Date: Thu, Mar 23, 2017 at 1:34 PM
Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
To: [hidden email]



Hi,

Please find the latest report on new defect(s) introduced to Apache
ActiveMQ Artemis found with Coverity Scan.

8 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
Scan.
8 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 8 of 8 defect(s)


** CID 1420682:  FindBugs: Correctness  (FB.GC_UNRELATED_TYPES)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallback.java:
252 in org.apache.activemq.artemis.protocol.amqp.broker.
AMQPConnectionCallback.removeTransaction(org.apache.
qpid.proton.amqp.Binary)()


____________________________________________________________
____________________________________________
*** CID 1420682:  FindBugs: Correctness  (FB.GC_UNRELATED_TYPES)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallback.java:
252 in org.apache.activemq.artemis.protocol.amqp.broker.
AMQPConnectionCallback.removeTransaction(org.apache.
qpid.proton.amqp.Binary)()
246
247           return tx;
248        }
249
250        public Transaction removeTransaction(Binary txid) {
251           XidImpl xid = newXID(txid.getArray());
>>>     CID 1420682:  FindBugs: Correctness  (FB.GC_UNRELATED_TYPES)
>>>     org.apache.activemq.artemis.core.transaction.impl.XidImpl is
incompatible with expected argument type org.apache.qpid.proton.amqp.Binary.
252           return transactions.remove(xid);
253        }
254
255        protected XidImpl newXID() {
256           return newXID(UUIDGenerator.getInstance().
generateStringUUID().getBytes());
257        }

** CID 1420681:  FindBugs: Correctness  (FB.EC_UNRELATED_TYPES)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/amqp/AmqpTransactionTest.java: 970 in
org.apache.activemq.artemis.tests.integration.amqp.AmqpTransactionTest$3.
inspectDeliveryUpdate(org.apache.qpid.proton.engine.Delivery)()


____________________________________________________________
____________________________________________
*** CID 1420681:  FindBugs: Correctness  (FB.EC_UNRELATED_TYPES)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/amqp/AmqpTransactionTest.java: 970 in
org.apache.activemq.artemis.tests.integration.amqp.AmqpTransactionTest$3.
inspectDeliveryUpdate(org.apache.qpid.proton.engine.Delivery)()
964                       }
965                    }
966
967                    TransactionalState localTxState =
(TransactionalState) delivery.getLocalState();
968                    TransactionalState remoteTxState =
(TransactionalState) delivery.getRemoteState();
969
>>>     CID 1420681:  FindBugs: Correctness  (FB.EC_UNRELATED_TYPES)
>>>     Call to org.apache.qpid.proton.amqp.Binary.equals(org.apache.qpid.
proton.amqp.transaction.TransactionalState).
970                    if (!localTxState.getTxnId().equals(remoteTxState)) {
971                       markAsInvalid("Message not enrolled in expected
transaction");
972                    }
973                 }
974              }
975           });

** CID 1420680:    (FB.DM_DEFAULT_ENCODING)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/mqtt/imported/MQTTTest.java: 202 in
org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.
testManagementQueueMessagesAreAckd()()
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/mqtt/imported/MQTTTest.java: 205 in
org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.
testManagementQueueMessagesAreAckd()()


____________________________________________________________
____________________________________________
*** CID 1420680:    (FB.DM_DEFAULT_ENCODING)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/mqtt/imported/MQTTTest.java: 202 in
org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.
testManagementQueueMessagesAreAckd()()
196           final MQTTClientProvider provider = getMQTTClientProvider();
197           provider.setClientId(clientId);
198           initializeConnection(provider);
199           provider.subscribe("foo", EXACTLY_ONCE);
200           for (int i = 0; i < NUM_MESSAGES; i++) {
201              String payload = "Test Message: " + i;
>>>     CID 1420680:    (FB.DM_DEFAULT_ENCODING)
>>>     Found reliance on default encoding: String.getBytes().
202              provider.publish("foo", payload.getBytes(), EXACTLY_ONCE);
203              byte[] message = provider.receive(5000);
204              assertNotNull("Should get a message", message);
205              assertEquals(payload, new String(message));
206           }
207
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/mqtt/imported/MQTTTest.java: 205 in
org.apache.activemq.artemis.tests.integration.mqtt.imported.MQTTTest.
testManagementQueueMessagesAreAckd()()
199           provider.subscribe("foo", EXACTLY_ONCE);
200           for (int i = 0; i < NUM_MESSAGES; i++) {
201              String payload = "Test Message: " + i;
202              provider.publish("foo", payload.getBytes(), EXACTLY_ONCE);
203              byte[] message = provider.receive(5000);
204              assertNotNull("Should get a message", message);
>>>     CID 1420680:    (FB.DM_DEFAULT_ENCODING)
>>>     Found reliance on default encoding: new String(byte[]).
205              assertEquals(payload, new String(message));
206           }
207
208           final Queue queue = server.locateQueue(new
SimpleString(MQTTUtil.MANAGEMENT_QUEUE_PREFIX + clientId));
209
210           Wait.waitFor(() -> queue.getMessageCount() == 0, 1000, 100);

** CID 1420679:  FindBugs: Performance  (FB.BX_UNBOXING_IMMEDIATELY_REBOXED)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java: 499 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.isDurable()()


____________________________________________________________
____________________________________________
*** CID 1420679:  FindBugs: Performance  (FB.BX_UNBOXING_IMMEDIATELY_
REBOXED)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java: 499 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.isDurable()()
493        public boolean isDurable() {
494           if (durable != null) {
495              return durable;
496           }
497
498           if (getHeader() != null && getHeader().getDurable() != null) {
>>>     CID 1420679:  FindBugs: Performance  (FB.BX_UNBOXING_IMMEDIATELY_
REBOXED)
>>>     Boxed value is unboxed and then immediately reboxed.
499              durable =  getHeader().getDurable().booleanValue();
500              return durable;
501           } else {
502              return durable != null ? durable : false;
503           }
504        }

** CID 1420678:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java: 159 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.
getApplicationProperties()()


____________________________________________________________
____________________________________________
*** CID 1420678:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java: 159 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.
getApplicationProperties()()
153        }
154
155        private ApplicationProperties getApplicationProperties() {
156           parseHeaders();
157
158           if (applicationProperties == null && appLocation >= 0) {
>>>     CID 1420678:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "getBuffer()".
159              ByteBuffer buffer = getBuffer().nioBuffer();
160              buffer.position(appLocation);
161              TLSEncode.getDecoder().setByteBuffer(buffer);
162              Object section = TLSEncode.getDecoder().readObject();
163              if (section instanceof ApplicationProperties) {
164                 this.applicationProperties = (ApplicationProperties)
section;

** CID 1420677:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java:
1291 in org.apache.activemq.artemis.core.server.impl.
ServerSessionImpl.send(org.apache.activemq.artemis.core.transaction.Transaction,
org.apache.activemq.artemis.api.core.Message, boolean, boolean)()


____________________________________________________________
____________________________________________
*** CID 1420677:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java:
1291 in org.apache.activemq.artemis.core.server.impl.
ServerSessionImpl.send(org.apache.activemq.artemis.core.transaction.Transaction,
org.apache.activemq.artemis.api.core.Message, boolean, boolean)()
1285        }
1286
1287        @Override
1288        public synchronized RoutingStatus send(Transaction tx,
1289                                               final Message message,
1290                                               final boolean direct,
>>>     CID 1420677:  Program hangs  (LOCK_INVERSION)
>>>     Acquiring lock "ServerSessionImpl.this".
1291                                               boolean
noAutoCreateQueue) throws Exception {
1292
1293           // If the protocol doesn't support flow control, we have no
choice other than fail the communication
1294           if (!this.getRemotingConnection().isSupportsFlowControl() &&
pagingManager.isDiskFull()) {
1295              ActiveMQIOErrorException exception =
ActiveMQMessageBundle.BUNDLE.diskBeyondLimit();
1296              this.getRemotingConnection().fail(exception);

** CID 1420676:    (DEADCODE)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/amqp/AmqpTestSupport.java: 71 in
org.apache.activemq.artemis.tests.integration.amqp.AmqpTestSupport.
getBrokerAmqpConnectionURI()()
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/amqp/AmqpTestSupport.java: 77 in
org.apache.activemq.artemis.tests.integration.amqp.AmqpTestSupport.
getBrokerAmqpConnectionURI()()


____________________________________________________________
____________________________________________
*** CID 1420676:    (DEADCODE)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/amqp/AmqpTestSupport.java: 71 in
org.apache.activemq.artemis.tests.integration.amqp.AmqpTestSupport.
getBrokerAmqpConnectionURI()()
65              int port = 61616;
66
67              String uri = null;
68
69              if (isUseSSL()) {
70                 if (webSocket) {
>>>     CID 1420676:    (DEADCODE)
>>>     Execution cannot reach this statement: "uri = "wss://127.0.0.1:" +
...".
71                    uri = "wss://127.0.0.1:" + port;
72                 } else {
73                    uri = "ssl://127.0.0.1:" + port;
74                 }
75              } else {
76                 if (webSocket) {
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/amqp/AmqpTestSupport.java: 77 in
org.apache.activemq.artemis.tests.integration.amqp.AmqpTestSupport.
getBrokerAmqpConnectionURI()()
71                    uri = "wss://127.0.0.1:" + port;
72                 } else {
73                    uri = "ssl://127.0.0.1:" + port;
74                 }
75              } else {
76                 if (webSocket) {
>>>     CID 1420676:    (DEADCODE)
>>>     Execution cannot reach this statement: "uri = "ws://127.0.0.1:" +
p...".
77                    uri = "ws://127.0.0.1:" + port;
78                 } else {
79                    uri = "tcp://127.0.0.1:" + port;
80                 }
81              }
82

** CID 1420675:  Class hierarchy inconsistencies  (CALL_SUPER)
/tests/artemis-test-support/src/main/java/org/apache/
activemq/transport/amqp/client/AmqpSender.java: 423 in
org.apache.activemq.transport.amqp.client.AmqpSender.
processDeliveryUpdates(org.apache.activemq.transport.amqp.client.AmqpConnection,
org.apache.qpid.proton.engine.Delivery)()


____________________________________________________________
____________________________________________
*** CID 1420675:  Class hierarchy inconsistencies  (CALL_SUPER)
/tests/artemis-test-support/src/main/java/org/apache/
activemq/transport/amqp/client/AmqpSender.java: 423 in
org.apache.activemq.transport.amqp.client.AmqpSender.
processDeliveryUpdates(org.apache.activemq.transport.amqp.client.AmqpConnection,
org.apache.qpid.proton.engine.Delivery)()
417                 LOG.warn("{} failed to send any data from current
Message.", this);
418              }
419           }
420        }
421
422        @Override
>>>     CID 1420675:  Class hierarchy inconsistencies  (CALL_SUPER)
>>>     Missing call to "org.apache.activemq.transport.amqp.client.
AmqpAbstractResource.processDeliveryUpdates(org.apache.activemq.transport.amqp.client.AmqpConnection,
org.apache.qpid.proton.engine.Delivery)" (as is done elsewhere 2 out of 3
times).
423        public void processDeliveryUpdates(AmqpConnection connection,
Delivery updated) throws IOException {
424           List<Delivery> toRemove = new ArrayList<>();
425
426           for (Delivery delivery : pending) {
427              DeliveryState state = delivery.getRemoteState();
428              if (state == null) {


____________________________________________________________
____________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.
net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-
2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70
GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wAcdjhhKQgo
A-2FzBgFriAc5uDVMa19pTXD-2FmYwa3DH3eK0-2By39FhpWAQKVPf8F02btjoXlBZq-
2BSkY69aFM3gWBNLd-2BFgo-2FcIigtMFD2-2BkCpIO86JMXS578-
2FyXoRCoiQ8XmHOwSsbM-2B-2BltqqYXBLrgH78Y8ncexHOrPOAduB0vca-2BzQ-3D-3D
--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

clebertsuconic
In reply to this post by Jiri Danek
Those are tests.  Can't we set an ignore on them ?
On Thu, Mar 23, 2017 at 8:40 AM Jiri Danek <[hidden email]> wrote:

>
> ---------- Forwarded message ----------
> From: <[hidden email]>
> Date: Fri, Mar 17, 2017 at 10:59 AM
> Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
> To: [hidden email]
>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Apache
> ActiveMQ Artemis found with Coverity Scan.
>
> 4 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
> Scan.
> 5 defect(s), reported by Coverity Scan earlier, were marked fixed in the
> recent build analyzed by Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 4 of 4 defect(s)
>
>
> ** CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
> /tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ConsumerTest.java:
> 419 in
> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
> int)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
> /tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ConsumerTest.java:
> 419 in
> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
> int)()
> 413
> 414              TextMessage msg = (TextMessage) consumer.receive(1000);
> 415              Assert.assertEquals("testSelectorExampleFromSpecs:2",
> msg.getText());
> 416
> 417           } finally {
> 418              connection.close();
> >>>     CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
> >>>     Variable "factorySend" going out of scope leaks the resource it
> refers to.
> 419           }
> 420        }
> 421
> 422        @Test
> 423        public void testConsumerAckImmediateAutoCommitTrue() throws
> Exception {
> 424           ClientSessionFactory sf = createSessionFactory(locator);
>
> ** CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
> /tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ConsumerTest.java:
> 317 in
> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
> int)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
> /tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ConsumerTest.java:
> 317 in
> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
> int)()
> 311        public void internalSend(int protocolSender, int
> protocolConsumer) throws Throwable {
> 312
> 313           ConnectionFactory factorySend =
> createFactory(protocolSender);
> 314           ConnectionFactory factoryConsume = protocolConsumer ==
> protocolSender ? factorySend : createFactory(protocolConsumer);
> 315
> 316
> >>>     CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
> >>>     Calling a method on null object "factorySend".
> 317           Connection connection = factorySend.createConnection();
> 318
> 319           try {
> 320              Session session = connection.createSession(false,
> Session.AUTO_ACKNOWLEDGE);
> 321              javax.jms.Queue queue =
> session.createQueue(QUEUE.toString());
> 322              MessageProducer producer = session.createProducer(queue);
>
> ** CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
> /artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java:
> 137 in org.apache.activemq.artemis.co
> re.paging.cursor.PagedReferenceImpl.getScheduledDeliveryTime()()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
> /artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java:
> 137 in org.apache.activemq.artemis.co
> re.paging.cursor.PagedReferenceImpl.getScheduledDeliveryTime()()
> 131
> 132        @Override
> 133        public long getScheduledDeliveryTime() {
> 134           if (deliveryTime == null) {
> 135              try {
> 136                 Message msg = getMessage();
> >>>     CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
> >>>     Unboxing null object "msg.getScheduledDeliveryTime()".
> 137                 return msg.getScheduledDeliveryTime();
> 138              } catch (Throwable e) {
> 139                 ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
> 140                 return 0L;
> 141              }
> 142           }
>
> ** CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
> /artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
> 2373 in org.apache.activemq.artemis.core.server.impl.QueueImpl.moveBetweenSnFQueues(org.apache.activemq.artemis.api.core.SimpleString,
> org.apache.activemq.artemis.core.transaction.Transaction,
> org.apache.activemq.artemis.core.server.MessageReference)()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
> /artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
> 2373 in org.apache.activemq.artemis.core.server.impl.QueueImpl.moveBetweenSnFQueues(org.apache.activemq.artemis.api.core.SimpleString,
> org.apache.activemq.artemis.core.transaction.Transaction,
> org.apache.activemq.artemis.core.server.MessageReference)()
> 2367           Binding targetBinding;
> 2368
> 2369           // remove the old route
> 2370           for (SimpleString propName :
> copyMessage.getPropertyNames()) {
> 2371              if (propName.startsWith(Message.HDR_ROUTE_TO_IDS)) {
> 2372                 oldRouteToIDs = (byte[])
> copyMessage.removeProperty(propName.toString());
> >>>     CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
> >>>     Calling a method on null object "oldRouteToIDs".
> 2373                 final String hashcodeToString =
> oldRouteToIDs.toString(); // don't use Arrays.toString(..) here
> 2374                 logger.debug("Removed property from message: " +
> propName + " = " + hashcodeToString + " (" +
> ByteBuffer.wrap(oldRouteToIDs).getLong() + ")");
> 2375
> 2376                 // there should only be one of these properties so
> potentially save some loop iterations
> 2377                 break;
> 2378              }
>
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit,
> https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wDEBs83dhsMiiHLs6eTC5vNN8SQrPHx5Y-2FunG8Ul9-2FtGelYwdL08hg4PK4L5N-2FrzHT6HG-2B7H7X8f0pdTqUNCmfC4-2BO8FqmU2U7GA8fkpPkZD935bABKyBgetbGXKZruAnUZrfa21ulwAIB5-2FFiJfax8q9bLx0-2B99BNXReKbArA-2BJg-3D-3D
>
--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
I think so, I clicked Ignore in analysis at
https://scan.coverity.com/projects/apache-activemq-artemis?tab=analysis_settings.
Hopefully that should do the trick.

On Thu, Mar 23, 2017 at 1:56 PM, Clebert Suconic <[hidden email]>
wrote:

> Those are tests.  Can't we set an ignore on them ?
>
> On Thu, Mar 23, 2017 at 8:40 AM Jiri Danek <[hidden email]> wrote:
>
>>
>> ---------- Forwarded message ----------
>> From: <[hidden email]>
>> Date: Fri, Mar 17, 2017 at 10:59 AM
>> Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
>> To: [hidden email]
>>
>>
>>
>> Hi,
>>
>> Please find the latest report on new defect(s) introduced to Apache
>> ActiveMQ Artemis found with Coverity Scan.
>>
>> 4 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
>> Scan.
>> 5 defect(s), reported by Coverity Scan earlier, were marked fixed in the
>> recent build analyzed by Coverity Scan.
>>
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 4 of 4 defect(s)
>>
>>
>> ** CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
>> /tests/integration-tests/src/test/java/org/apache/activemq/
>> artemis/tests/integration/client/ConsumerTest.java: 419 in
>> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
>> int)()
>>
>>
>> ____________________________________________________________
>> ____________________________________________
>> *** CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
>> /tests/integration-tests/src/test/java/org/apache/activemq/
>> artemis/tests/integration/client/ConsumerTest.java: 419 in
>> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
>> int)()
>> 413
>> 414              TextMessage msg = (TextMessage) consumer.receive(1000);
>> 415              Assert.assertEquals("testSelectorExampleFromSpecs:2",
>> msg.getText());
>> 416
>> 417           } finally {
>> 418              connection.close();
>> >>>     CID 1418581:  Exceptional resource leaks  (RESOURCE_LEAK)
>> >>>     Variable "factorySend" going out of scope leaks the resource it
>> refers to.
>> 419           }
>> 420        }
>> 421
>> 422        @Test
>> 423        public void testConsumerAckImmediateAutoCommitTrue() throws
>> Exception {
>> 424           ClientSessionFactory sf = createSessionFactory(locator);
>>
>> ** CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
>> /tests/integration-tests/src/test/java/org/apache/activemq/
>> artemis/tests/integration/client/ConsumerTest.java: 317 in
>> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
>> int)()
>>
>>
>> ____________________________________________________________
>> ____________________________________________
>> *** CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
>> /tests/integration-tests/src/test/java/org/apache/activemq/
>> artemis/tests/integration/client/ConsumerTest.java: 317 in
>> org.apache.activemq.artemis.tests.integration.client.ConsumerTest.internalSend(int,
>> int)()
>> 311        public void internalSend(int protocolSender, int
>> protocolConsumer) throws Throwable {
>> 312
>> 313           ConnectionFactory factorySend =
>> createFactory(protocolSender);
>> 314           ConnectionFactory factoryConsume = protocolConsumer ==
>> protocolSender ? factorySend : createFactory(protocolConsumer);
>> 315
>> 316
>> >>>     CID 1418580:  Null pointer dereferences  (NULL_RETURNS)
>> >>>     Calling a method on null object "factorySend".
>> 317           Connection connection = factorySend.createConnection();
>> 318
>> 319           try {
>> 320              Session session = connection.createSession(false,
>> Session.AUTO_ACKNOWLEDGE);
>> 321              javax.jms.Queue queue = session.createQueue(QUEUE.
>> toString());
>> 322              MessageProducer producer = session.createProducer(queue);
>>
>> ** CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
>> /artemis-server/src/main/java/org/apache/activemq/artemis/
>> core/paging/cursor/PagedReferenceImpl.java: 137 in
>> org.apache.activemq.artemis.core.paging.cursor.PagedReferenceImpl.
>> getScheduledDeliveryTime()()
>>
>>
>> ____________________________________________________________
>> ____________________________________________
>> *** CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
>> /artemis-server/src/main/java/org/apache/activemq/artemis/
>> core/paging/cursor/PagedReferenceImpl.java: 137 in
>> org.apache.activemq.artemis.core.paging.cursor.PagedReferenceImpl.
>> getScheduledDeliveryTime()()
>> 131
>> 132        @Override
>> 133        public long getScheduledDeliveryTime() {
>> 134           if (deliveryTime == null) {
>> 135              try {
>> 136                 Message msg = getMessage();
>> >>>     CID 1418579:  Null pointer dereferences  (NULL_RETURNS)
>> >>>     Unboxing null object "msg.getScheduledDeliveryTime()".
>> 137                 return msg.getScheduledDeliveryTime();
>> 138              } catch (Throwable e) {
>> 139                 ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
>> 140                 return 0L;
>> 141              }
>> 142           }
>>
>> ** CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
>> /artemis-server/src/main/java/org/apache/activemq/artemis/
>> core/server/impl/QueueImpl.java: 2373 in org.apache.activemq.artemis.co
>> re.server.impl.QueueImpl.moveBetweenSnFQueues(org.
>> apache.activemq.artemis.api.core.SimpleString,
>> org.apache.activemq.artemis.core.transaction.Transaction,
>> org.apache.activemq.artemis.core.server.MessageReference)()
>>
>>
>> ____________________________________________________________
>> ____________________________________________
>> *** CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
>> /artemis-server/src/main/java/org/apache/activemq/artemis/
>> core/server/impl/QueueImpl.java: 2373 in org.apache.activemq.artemis.co
>> re.server.impl.QueueImpl.moveBetweenSnFQueues(org.
>> apache.activemq.artemis.api.core.SimpleString,
>> org.apache.activemq.artemis.core.transaction.Transaction,
>> org.apache.activemq.artemis.core.server.MessageReference)()
>> 2367           Binding targetBinding;
>> 2368
>> 2369           // remove the old route
>> 2370           for (SimpleString propName : copyMessage.getPropertyNames())
>> {
>> 2371              if (propName.startsWith(Message.HDR_ROUTE_TO_IDS)) {
>> 2372                 oldRouteToIDs = (byte[]) copyMessage.removeProperty(
>> propName.toString());
>> >>>     CID 1418578:  Null pointer dereferences  (NULL_RETURNS)
>> >>>     Calling a method on null object "oldRouteToIDs".
>> 2373                 final String hashcodeToString =
>> oldRouteToIDs.toString(); // don't use Arrays.toString(..) here
>> 2374                 logger.debug("Removed property from message: " +
>> propName + " = " + hashcodeToString + " (" + ByteBuffer.wrap(oldRouteToIDs).getLong()
>> + ")");
>> 2375
>> 2376                 // there should only be one of these properties so
>> potentially save some loop iterations
>> 2377                 break;
>> 2378              }
>>
>>
>> ____________________________________________________________
>> ____________________________________________
>> To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.
>> net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-
>> 2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70
>> GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wDEBs83dhsM
>> iiHLs6eTC5vNN8SQrPHx5Y-2FunG8Ul9-2FtGelYwdL08hg4PK4L5N-2FrzHT6HG-
>> 2B7H7X8f0pdTqUNCmfC4-2BO8FqmU2U7GA8fkpPkZD935bABKyB
>> getbGXKZruAnUZrfa21ulwAIB5-2FFiJfax8q9bLx0-2B99BNXReKbArA-2BJg-3D-3D
>>
> --
> Clebert Suconic
>



--
Jiří Daněk
Messaging QA
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
Apparently, the filter I set only filters it in the Web UI. To filter out
tests for purpose of e-mail notification, I have to explicitly enable what
I care about.
This time I went through the report manually and deleted the 4 instances
reported in test code and examples. I also removed two finds which are
obviously false positives. They are at the end


---------- Forwarded message ----------
From: <[hidden email]>
Date: Sat, Apr 1, 2017 at 3:49 PM
Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
To: [hidden email]



Hi,

Please find the latest report on new defect(s) introduced to Apache
ActiveMQ Artemis found with Coverity Scan.

10 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
Scan.
6 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 10 of 10 defect(s)


** CID 1422915:  Null pointer dereferences  (FORWARD_NULL)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java: 283 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.setRoutingType(
org.apache.activemq.artemis.api.core.RoutingType)()


____________________________________________________________
____________________________________________
*** CID 1422915:  Null pointer dereferences  (FORWARD_NULL)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java: 283 in
org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage.setRoutingType(
org.apache.activemq.artemis.api.core.RoutingType)()
277        @Override
278        public org.apache.activemq.artemis.api.core.Message
setRoutingType(RoutingType routingType) {
279           parseHeaders();
280           if (routingType == null) {
281              removeSymbol(AMQPMessageSupport.ROUTING_TYPE);
282           }
>>>     CID 1422915:  Null pointer dereferences  (FORWARD_NULL)
>>>     Calling a method on null object "routingType".
283           setSymbol(AMQPMessageSupport.ROUTING_TYPE,
routingType.getType());
284           return this;
285        }
286
287        @Override
288        public SimpleString getGroupID() {

____________________________________________________________
____________________________________________
*** CID 1422918:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/
PagingStoreFactoryDatabase.java: 89 in org.apache.activemq.artemis.
core.paging.impl.PagingStoreFactoryDatabase.newExecutor()()
83        public ScheduledExecutorService getScheduledExecutor() {
84           return scheduledExecutor;
85        }
86
87        @Override
88        public Executor newExecutor() {
>>>     CID 1422918:  Concurrent data access violations
(GUARDED_BY_VIOLATION)
>>>     Accessing "executorFactory" without holding lock
"PagingStoreFactoryDatabase.this". Elsewhere, "org.apache.activemq.artemis.
core.paging.impl.PagingStoreFactoryDatabase.executorFactory" is accessed
with "PagingStoreFactoryDatabase.this" held 5 out of 6 times.
89           return executorFactory.getExecutor();
90        }
91
92        private boolean started = false;
93
94        public PagingStoreFactoryDatabase(final
DatabaseStorageConfiguration dbConf,

** CID 1422919:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/
ActiveMQThreadPoolExecutor.java: 60 in org.apache.activemq.artemis.utils.
ActiveMQThreadPoolExecutor$ThreadPoolQueue.setExecutor(
org.apache.activemq.artemis.utils.ActiveMQThreadPoolExecutor)()


____________________________________________________________
____________________________________________
*** CID 1422919:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/
ActiveMQThreadPoolExecutor.java: 60 in org.apache.activemq.artemis.utils.
ActiveMQThreadPoolExecutor$ThreadPoolQueue.setExecutor(
org.apache.activemq.artemis.utils.ActiveMQThreadPoolExecutor)()
54           // idle threads than queued tasks and can add more tasks into
the queue.
55           // The delta is incremented if a thread becomes idle or if a
task is taken from the queue.
56           // The delta is decremented if a thread leaves idle state or
if a task is added to the queue.
57           private int threadTaskDelta = 0;
58
59           public void setExecutor(ActiveMQThreadPoolExecutor executor) {
>>>     CID 1422919:  Concurrent data access violations
(GUARDED_BY_VIOLATION)
>>>     Accessing "this.executor" without holding lock
"ThreadPoolQueue.lock". Elsewhere, "org.apache.activemq.artemis.utils.
ActiveMQThreadPoolExecutor$ThreadPoolQueue.executor" is accessed with
"ThreadPoolQueue.lock" held 2 out of 3 times.
60              this.executor = executor;
61           }
62
63           @Override
64           public boolean offer(Runnable runnable) {
65              boolean retval = false;

** CID 1422920:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/proton/handler/ProtonHandler.java:
156 in org.apache.activemq.artemis.protocol.amqp.proton.handler.
ProtonHandler.flushBytes()()


____________________________________________________________
____________________________________________
*** CID 1422920:  Null pointer dereferences  (NULL_RETURNS)
/artemis-protocols/artemis-amqp-protocol/src/main/java/
org/apache/activemq/artemis/protocol/amqp/proton/handler/ProtonHandler.java:
156 in org.apache.activemq.artemis.protocol.amqp.proton.handler.
ProtonHandler.flushBytes()()
150                    break;
151                 }
152
153                 // We allocated a Pooled Direct Buffer, that will be
sent down the stream
154                 ByteBuf buffer = PooledByteBufAllocator.
DEFAULT.directBuffer(pending);
155                 ByteBuffer head = transport.head();
>>>     CID 1422920:  Null pointer dereferences  (NULL_RETURNS)
>>>     Dereferencing a pointer that might be null "head" when calling
"writeBytes". (The virtual call resolves to "io.netty.buffer.
AbstractByteBuf.writeBytes".)
156                 buffer.writeBytes(head);
157
158                 for (EventHandler handler : handlers) {
159                    handler.pushBytes(buffer);
160                 }
161

___________________________________________________________
____________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.
net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-
2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70
GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wAoFGpCKXPq
Hg46Wuh9HLHsluwhatflFRLFlJhuPbFONE5iUGlirP6eVdPuDLIDLC0TsnhdcMbXqfo6LoIMxF0-
2FR2IhWwHvgTSWN-2FhtsoXXoY38utbJq9RpdAfVowdqLGqwvy3-
2BC7HcpYGA9ksz0rChzXs8G4YA-2BBC7eJZCyEjUhg-3D-3D


And the false positives

** CID 1422916:  Null pointer dereferences  (FORWARD_NULL)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/openwire/amq/ProducerFlowControlBaseTest.java: 60
in org.apache.activemq.artemis.tests.integration.openwire.amq.
ProducerFlowControlBaseTest$1.run()()


____________________________________________________________
____________________________________________
*** CID 1422916:  Null pointer dereferences  (FORWARD_NULL)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/openwire/amq/ProducerFlowControlBaseTest.java: 60
in org.apache.activemq.artemis.tests.integration.openwire.amq.
ProducerFlowControlBaseTest$1.run()()
54              // flag to false.
55              // Once the send starts to block it will not reset the done
flag
56              // anymore.
57              asyncThread = new Thread("Fill thread.") {
58                 @Override
59                 public void run() {
>>>     CID 1422916:  Null pointer dereferences  (FORWARD_NULL)
>>>     Assigning: "session" = "null".
60                    Session session = null;
61                    try {
62                       session = flowControlConnection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
63                       MessageProducer producer =
session.createProducer(queue);
64                       producer.setDeliveryMode(
DeliveryMode.NON_PERSISTENT);
65                       while (keepGoing.get()) {

** CID 1422917:  Null pointer dereferences  (FORWARD_NULL)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/openwire/amq/ProducerFlowControlBaseTest.java:
100 in org.apache.activemq.artemis.tests.integration.openwire.amq.
ProducerFlowControlBaseTest$2.run()()


____________________________________________________________
____________________________________________
*** CID 1422917:  Null pointer dereferences  (FORWARD_NULL)
/tests/integration-tests/src/test/java/org/apache/activemq/
artemis/tests/integration/openwire/amq/ProducerFlowControlBaseTest.java:
100 in org.apache.activemq.artemis.tests.integration.openwire.amq.
ProducerFlowControlBaseTest$2.run()()
94
95        protected CountDownLatch asyncSendTo(final ActiveMQQueue queue,
final String message) throws JMSException {
96           final CountDownLatch done = new CountDownLatch(1);
97           new Thread("Send thread.") {
98              @Override
99              public void run() {
>>>     CID 1422917:  Null pointer dereferences  (FORWARD_NULL)
>>>     Assigning: "session" = "null".
100                 Session session = null;
101                 try {
102                    session = flowControlConnection.createSession(false,
Session.AUTO_ACKNOWLEDGE);
103                    MessageProducer producer =
session.createProducer(queue);
104                    producer.setDeliveryMode(
DeliveryMode.NON_PERSISTENT);
105                    producer.send(session.createTextMessage(message));

** CID 1422918:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/
PagingStoreFactoryDatabase.java: 89 in org.apache.activemq.artemis.
core.paging.impl.PagingStoreFactoryDatabase.newExecutor()()

--
Jiří Daněk
Messaging QA
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
Hi, new results for Coverity Scan are here.

Before, though, I would like to give honorable mention to thing from
Coverity Scan backlog which was found some time ago and never was in these
e-mails of mine

 82   boolean addSubscription(MqttTopicSubscription subscription,
WildcardConfiguration wildcardConfiguration) {

CID 1419151 (#1 of 1): JLM: Synchronization on java.util.concurrent objects
(FB.JLM_JSR166_UTILCONCURRENT_MONITORENTER)
1. defect: Synchronization performed on java.util.concurrent.Concurren
tHashMap.
 83      synchronized (subscriptions) {
 84         addressMessageMap.putIfAbsent(MQTTUtil.convertMQTTAddressFil
terToCore(subscription.topicName(), wildcardConfiguration), new
ConcurrentHashMap<Long, Integer>());

According to http://www.cs.umd.edu/~pugh/MistakesThatMatter.pdf slides
32,33 this is almost always a bug and it was rather prevalent in JBoss...

Another instance of this

100   void removeSubscription(String address) {

CID 1419085 (#1 of 1): JLM: Synchronization on java.util.concurrent objects
(FB.JLM_JSR166_UTILCONCURRENT_MONITORENTER)1. defect: Synchronization
performed on java.util.concurrent.ConcurrentHashMap.
101      synchronized (subscriptions) {
102         subscriptions.remove(address);
103         addressMessageMap.remove(address);
104      }
105   }


Hi,

Please find the latest report on new defect(s) introduced to Apache
ActiveMQ Artemis found with Coverity Scan.

1 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
Scan.


New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 1426233:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java:
399 in org.apache.activemq.artemis.core.replication.ReplicationManager.
flowControl()()

9. lock_order: Acquiring lock PagingStoreImpl.lock while holding
ReplicationManager.replicationLock conflicts with the lock order
established elsewhere. [show details
<https://scan7.coverity.com/eventId=3237539-10&modelId=3237539-0&fileInstanceId=13594886&filePath=%2Fartemis-server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Factivemq%2Fartemis%2Fcore%2Freplication%2FReplicationManager.java&fileStart=281&fileEnd=308>
]

____________________________________________________________
____________________________________________
*** CID 1426233:  Program hangs  (LOCK_INVERSION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java:
399 in org.apache.activemq.artemis.core.replication.ReplicationManager.
flowControl()()
393
394        /**
395         * This was written as a refactoring of sendReplicatePacket.
396         * In case you refactor this in any way, this method must hold a
lock on replication lock. .
397         */
398        private boolean flowControl() {
>>>     CID 1426233:  Program hangs  (LOCK_INVERSION)
>>>     Acquiring lock "ReplicationManager.replicationLock".
399           synchronized (replicationLock) {
400              // synchronized (replicationLock) { -- I'm not adding this
because the caller already has it
401              // future maintainers of this code please be aware that
the intention here is hold the lock on replication lock
402              if (!replicatingChannel.getConnection().isWritable(this)) {
403                 try {
404                    logger.trace("flowControl waiting on writable
replication");

https://scan7.coverity.com/reports.htm#v10043/p14213/
fileInstanceId=13594886&defectInstanceId=3237539&mergedDefectId=1426233

--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
New Coverity report is here. I deleted three finds which were in test code.

---------- Forwarded message ----------
From: <[hidden email]>
Date: Wed, Apr 12, 2017 at 3:59 PM
Subject: New Defects reported by Coverity Scan for Apache ActiveMQ Artemis
To: [hidden email]



Hi,

Please find the latest report on new defect(s) introduced to Apache
ActiveMQ Artemis found with Coverity Scan.

7 new defect(s) introduced to Apache ActiveMQ Artemis found with Coverity
Scan.
4 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 7 of 7 defect(s)


** CID 1428142:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/
NamedLiveNodeLocatorForReplication.java: 108 in org.apache.activemq.artemis.
core.server.impl.NamedLiveNodeLocatorForReplication.getLiveConfiguration()()


____________________________________________________________
____________________________________________
*** CID 1428142:  Concurrent data access violations  (GUARDED_BY_VIOLATION)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/
NamedLiveNodeLocatorForReplication.java: 108 in org.apache.activemq.artemis.
core.server.impl.NamedLiveNodeLocatorForReplication.getLiveConfiguration()()
102        public String getNodeID() {
103           return nodeID;
104        }
105
106        @Override
107        public Pair<TransportConfiguration, TransportConfiguration>
getLiveConfiguration() {
>>>     CID 1428142:  Concurrent data access violations
(GUARDED_BY_VIOLATION)
>>>     Accessing "liveConfigurations" without holding lock "
NamedLiveNodeLocatorForReplication.lock.lock()". Elsewhere,
"org.apache.activemq.artemis.core.server.impl.NamedLiveNodeLocatorForReplication.liveConfigurations"
is accessed with "NamedLiveNodeLocatorForReplication.lock.lock()" held 5
out of 6 times.
108           return liveConfigurations.peek();
109        }
110
111        @Override
112        public void notifyRegistrationFailed(boolean alreadyReplicating)
{
113           try {

** CID 1428143:  Null pointer dereferences  (NULL_RETURNS)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/
SharedNothingBackupActivation.java: 211 in org.apache.activemq.artemis.
core.server.impl.SharedNothingBackupActivation.run()()


____________________________________________________________
____________________________________________
*** CID 1428143:  Null pointer dereferences  (NULL_RETURNS)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/
SharedNothingBackupActivation.java: 211 in org.apache.activemq.artemis.
core.server.impl.SharedNothingBackupActivation.run()()
205                    }
206                    activeMQServer.getNodeManager().setNodeID(nodeID);
207                 }
208
209                 try {
210                    if (logger.isTraceEnabled()) {
>>>     CID 1428143:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "possibleLive".
211                       logger.trace("Calling clusterController.
connectToNodeInReplicatedCluster(" + possibleLive.getA() + ")");
212                    }
213                    clusterControl = clusterController.
connectToNodeInReplicatedCluster(possibleLive.getA());
214                 } catch (Exception e) {
215                    logger.debug(e.getMessage(), e);
216                    if (possibleLive.getB() != null) {


____________________________________________________________
____________________________________________
*** CID 1428146:  FindBugs: Correctness  (FB.EC_UNRELATED_TYPES)
/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
720 in org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.
bindingQuery(org.apache.activemq.artemis.api.core.SimpleString)()
714
715           List<SimpleString> names = new ArrayList<>();
716
717           // make an exception for the management address (see
HORNETQ-29)
718           ManagementService managementService = getManagementService();
719           if (managementService != null) {
>>>     CID 1428146:  FindBugs: Correctness  (FB.EC_UNRELATED_TYPES)
>>>     Call to String.equals(org.apache.activemq.artemis.api.core.
SimpleString).
720              if
(realAddress.equals(managementService.getManagementAddress()))
{
721                 return new BindingQueryResult(true, names,
autoCreateQeueus, autoCreateAddresses, defaultPurgeOnNoConsumers,
defaultMaxConsumers);
722              }
723           }
724
725           SimpleString bindAddress = new SimpleString(realAddress);



____________________________________________________________
____________________________________________
*** CID 1428148:  FindBugs: Bad practice  (FB.RV_RETURN_VALUE_IGNORED_
BAD_PRACTICE)
/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/FileBroker.java:
122 in org.apache.activemq.artemis.integration.FileBroker.
createDirectories(org.apache.activemq.artemis.core.config.
impl.FileConfiguration)()
116
117
118        private void createDirectories(FileConfiguration
fileConfiguration) {
119           fileConfiguration.getPagingLocation().mkdirs();
120           fileConfiguration.getJournalLocation().mkdirs();
121           fileConfiguration.getBindingsLocation().mkdirs();
>>>     CID 1428148:  FindBugs: Bad practice  (FB.RV_RETURN_VALUE_IGNORED_
BAD_PRACTICE)
>>>     Another occurrence here
122           fileConfiguration.getLargeMessagesLocation().mkdirs();
123        }
124
125        @Override
126        public void stop() throws Exception {
127           stop(false);


____________________________________________________________
____________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.
net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-
2Bd2MGckcRZSbhom32dlDl11LWEm9nX1rtAWaC-2BDSVBpUSy28m9Zb8yC8TsR8PEkb70
GF-2BiZPHs-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wB7RF8hxWs3
bmvVaFgFAQvTuBGT0hrZhYWvUHIQlXAXi0Yw2XQOZsASFaAcWm-
2FLi8xJbNsE9vdgIrBAhe3KnioFEN9XlpBF2wV1Soc4uSGMzMBrezMv5K8vd
bliXxEvK4-2Fyhum-2Bc9-2FJJKbxyddiMo-2F7ZeBM-2B-2BFCXTMjCI1YGt113g-3D-3D

--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
New Coverity Scan results are here. Before posting that, I decided to also
make a list of previously reported items that were fixed during the course
of the week.

The following six Coverity Scan items were fixed or somehow removed during
the last week. They are listed together with commit that removed them.

The interesting thing here could be the 'Revert "ARTEMIS-1093 Full
qualified queue name support"' commit, I think.

ARTEMIS-1089 Improving flow control on replication:

    CID 1426233 (#1 of 1): Thread deadlock (LOCK_INVERSION)
    9. lock_order: Acquiring lock PagingStoreImpl.lock while holding
ReplicationManager.replicationLock conflicts with the lock order
established elsewhere. [show details]

ARTEMIS-1096 Changing Global Max Size's default:
    CID 1419093 (#1 of 1): RV: Bad use of return value
(FB.RV_RETURN_VALUE_IGNORED_BAD_PRACTICE)
    1. defect: Exceptional return value of java.io.File.mkdirs() ignored.

    (the code was only changed to form that either Coverity no longer
recognizes for purposes of the return-value-ignored warning, or the changes
caused Coverity to close this warning and create new one)

    CID 1418686 (#1 of 1): DLS: Dead local store (FB.DLS_DEAD_LOCAL_STORE)
    1. defect: Dead store to fileConfiguration.

ARTEMIS-1094 replica + group-name fix:

    CID 1409075 (#1 of 1): Unguarded read (GUARDED_BY_VIOLATION)
    1. missing_lock: Accessing liveConfiguration without holding lock
NamedLiveNodeLocatorForReplication.lock.lock(). Elsewhere,
"org.apache.activemq.artemis.core.server.impl.NamedLiveNodeLocatorForReplication.liveConfiguration"
is accessed with NamedLiveNodeLocatorForReplication.lock.lock() held 4 out
of 5 times.

Revert "ARTEMIS-1093 Full qualified queue name support"

    CID 1428146 (#1 of 1): EC: Comparing incompatible types for equality
(FB.EC_UNRELATED_TYPES)
    1. defect: Call to
String.equals(org.apache.activemq.artemis.api.core.SimpleString).

ARTEMIS-1108: Removed AIOFileLockManager:

    CID 1418595 (#1 of 1): DLS: Dead local store (FB.DLS_DEAD_LOCAL_STORE)
    1. defect: Dead store to file.

--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
As always, I am deleting the finds in test code. This time there were two
such.

---------- Forwarded message ----------
From: <[hidden email]>
Date: Fri, Apr 14, 2017 at 6:28 PM
Subject: New Defects reported by Coverity Scan for ApacheActiveMQArtemis
To: [hidden email]



Hi,

Please find the latest report on new defect(s) introduced to
ApacheActiveMQArtemis found with Coverity Scan.

3 new defect(s) introduced to ApacheActiveMQArtemis found with Coverity
Scan.
2 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 3 of 3 defect(s)


** CID 1428594:  FindBugs: Multithreaded correctness  (FB.JLM_JSR166_LOCK_
MONITORENTER)
/activemq-artemis/artemis-protocols/artemis-amqp-protocol/src/main/java/org/
apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java: 485
in org.apache.activemq.artemis.protocol.amqp.broker.
AMQPSessionCallback$4.run()()


____________________________________________________________
____________________________________________
*** CID 1428594:  FindBugs: Multithreaded correctness  (FB.JLM_JSR166_LOCK_
MONITORENTER)
/activemq-artemis/artemis-protocols/artemis-amqp-protocol/src/main/java/org/
apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java: 485
in org.apache.activemq.artemis.protocol.amqp.broker.
AMQPSessionCallback$4.run()()
479                 return;
480              }
481              final PagingStore store = manager.getServer().
getPagingManager().getPageStore(new SimpleString(address));
482              store.checkMemory(new Runnable() {
483                 @Override
484                 public void run() {
>>>     CID 1428594:  FindBugs: Multithreaded correctness
(FB.JLM_JSR166_LOCK_MONITORENTER)
>>>     Synchronization performed on java.util.concurrent.locks.
ReentrantLock.
485                    synchronized (connection.getLock()) {
486                       if (receiver.getRemoteCredit() <= threshold) {
487                          receiver.flow(credits);
488                          connection.flush();
489                       }
490                    }


____________________________________________________________
____________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.
net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-
2Bd2MGckcRZSbhom32dlDl11LWEm9nX1lp9PjHD09mahOUNyqjCW7QTbr6zO
0eKYaenYqtNdjSI-3D_FskC5xBa3KJMdLzpQ7DMPdi-2Bg7iORJg0iEJDpvzM9wD9Z1wPDnXs
pCs2qaCwrT-2F8pUL-2BEy72Q4WB595-2BHjbSUcrDbJQiq1a6aQTyGWEvTg6Q
6jzIeXF0HN6mYT1Hv82GcBOjJ4-2FZ7YWX2zdE-2FRg8clR4DD6ot4sHUjSBn3d-
2Focs0OtAakzs8GZ-2Fy0LVB1CQ20b-2B-2BYfBskkjawotDNlenmA-3D-3D

--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
One last thing, I've created a scheduled job on TravisCI which will run
Coverity Scan automatically on a daily basis. Check it out at
https://github.com/msgqe/travisci/tree/activemq-artemis-coverity

This should allow doing the scan automatically in an open way that anybody
can look at, instead of me having to do it manually all the time, which is
unsustainable.

Project on Coverity Scan had to be renamed in order for the import from
Travis to work. The spaces had to be removed, So it is now at
https://scan.coverity.com/projects/apacheactivemqartemis, instead of
apache-activemq-artemis.

From Travis build initiation to Coverity Scan analysis being complete it
takes about 30 minutes.

(I picked Travis just because it is easiest to set up. I am open to
changes.)
--
Jiří Daněk
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Coverity Scan for Artemis

Jiri Danek
First result from the Travis setup. Zero new issues, and one previous find
was dealt with.

The one issue was fixed in commit ARTEMIS-1111 Fixing deadlock


CID 1428594 (#1 of 1): JLM: Synchronization on java.util.concurrent objects
(FB.JLM_JSR166_LOCK_MONITORENTER)1. defect: Synchronization performed on
java.util.concurrent.locks.ReentrantLock.

Cheers,
--
Jiří Daněk