You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Maxim Muzafarov <ma...@gmail.com> on 2018/08/01 08:35:41 UTC

Re: find bugs code check

Hello,

I can be wrong, but looks like -

`GridIoManager` not a bug, we are checking isEmpty here.
`GridCacheLockImpl` not a bug, variable is volatile.

Suppose, null-check not too critial for these classes, but can be done.

`GridTuple6` - must be fixed.
`GridClientJdkMarshaller`  - should be fixed.

On Tue, 31 Jul 2018 at 16:44 Евгений Станиловский
<ar...@mail.ru.invalid> wrote:

> hi, igniters.
> I take a little time to analyze findbugs  http://findbugs.sourceforge.net/
>  output from ignite-core module.
> There is summary of suspicious messages:
>
> GridIoManager
>
> sync on conc map:
>
> synchronized (map) {
> rmv = map.remove(set.nodeId(), set);
> }
> ---------------
>
> GridCacheLockImpl
>
> read without sync monitor
>
> final int getPermits() {
> return getState();
> }
>
> final synchronized void setPermits(int permits) {
> setState(permits);
> }
>
> -----------------------
>
> GridDhtPartitionFullMap
>
> add null check
>
> @Override public boolean equals(Object o) {
> if (this == o)
> return true;
>
> GridDhtPartitionFullMap other = (GridDhtPartitionFullMap)o;
>
> return other.nodeId.equals(nodeId) && other.updateSeq == updateSeq;
> }
>
> GridDhtPartitionMap
>
> add null check
>
> @Override public boolean equals(Object o) {
> if (this == o)
> return true;
>
> GridDhtPartitionMap other = (GridDhtPartitionMap)o;
>
> return other.nodeId.equals(nodeId) && other.updateSeq == updateSeq;
> }
>
> add null check
>
> GridNearOptimisticTxPrepareFuture
>
> @Override public boolean equals(Object o) {
> MappingKey that = (MappingKey) o;
>
> return nearEntries == that.nearEntries && nodeId.equals(that.nodeId);
> }
>
> -----------------
>
> copy-paste:
> public class GridTuple6
>
> @Override public boolean equals(Object o) {
> if (this == o)
> return true;
>
> if (!(o instanceof GridTuple5))
> return false;
>
>
> -------------------
>
> not closing stream:
> public class GridClientJdkMarshaller implements GridClientMarshaller {
> /** ID. */
> public static final byte ID = 2;
>
> /** {@inheritDoc} */
> @Override public ByteBuffer marshal(Object obj, int off) throws
> IOException {
> GridByteArrayOutputStream bOut = new GridByteArrayOutputStream();
>
> ObjectOutput out = new ObjectOutputStream(bOut);
> plz take a look on it, thanks !

-- 
--
Maxim Muzafarov

Re: find bugs code check

Posted by Dmitriy Pavlov <dp...@gmail.com>.
Hi Maxim, Evgeniy,

Thank you for researching and finding these issues.

Who could create tickets (probably, newbie) so newcomers can pick up and
fix it?

Sincerely,
Dmitriy Pavlov

ср, 1 авг. 2018 г. в 11:36, Maxim Muzafarov <ma...@gmail.com>:

> Hello,
>
> I can be wrong, but looks like -
>
> `GridIoManager` not a bug, we are checking isEmpty here.
> `GridCacheLockImpl` not a bug, variable is volatile.
>
> Suppose, null-check not too critial for these classes, but can be done.
>
> `GridTuple6` - must be fixed.
> `GridClientJdkMarshaller`  - should be fixed.
>
> On Tue, 31 Jul 2018 at 16:44 Евгений Станиловский
> <ar...@mail.ru.invalid> wrote:
>
> > hi, igniters.
> > I take a little time to analyze findbugs
> http://findbugs.sourceforge.net/
> >  output from ignite-core module.
> > There is summary of suspicious messages:
> >
> > GridIoManager
> >
> > sync on conc map:
> >
> > synchronized (map) {
> > rmv = map.remove(set.nodeId(), set);
> > }
> > ---------------
> >
> > GridCacheLockImpl
> >
> > read without sync monitor
> >
> > final int getPermits() {
> > return getState();
> > }
> >
> > final synchronized void setPermits(int permits) {
> > setState(permits);
> > }
> >
> > -----------------------
> >
> > GridDhtPartitionFullMap
> >
> > add null check
> >
> > @Override public boolean equals(Object o) {
> > if (this == o)
> > return true;
> >
> > GridDhtPartitionFullMap other = (GridDhtPartitionFullMap)o;
> >
> > return other.nodeId.equals(nodeId) && other.updateSeq == updateSeq;
> > }
> >
> > GridDhtPartitionMap
> >
> > add null check
> >
> > @Override public boolean equals(Object o) {
> > if (this == o)
> > return true;
> >
> > GridDhtPartitionMap other = (GridDhtPartitionMap)o;
> >
> > return other.nodeId.equals(nodeId) && other.updateSeq == updateSeq;
> > }
> >
> > add null check
> >
> > GridNearOptimisticTxPrepareFuture
> >
> > @Override public boolean equals(Object o) {
> > MappingKey that = (MappingKey) o;
> >
> > return nearEntries == that.nearEntries && nodeId.equals(that.nodeId);
> > }
> >
> > -----------------
> >
> > copy-paste:
> > public class GridTuple6
> >
> > @Override public boolean equals(Object o) {
> > if (this == o)
> > return true;
> >
> > if (!(o instanceof GridTuple5))
> > return false;
> >
> >
> > -------------------
> >
> > not closing stream:
> > public class GridClientJdkMarshaller implements GridClientMarshaller {
> > /** ID. */
> > public static final byte ID = 2;
> >
> > /** {@inheritDoc} */
> > @Override public ByteBuffer marshal(Object obj, int off) throws
> > IOException {
> > GridByteArrayOutputStream bOut = new GridByteArrayOutputStream();
> >
> > ObjectOutput out = new ObjectOutputStream(bOut);
> > plz take a look on it, thanks !
>
> --
> --
> Maxim Muzafarov
>