You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Евгений Станиловский <ar...@mail.ru.INVALID> on 2018/07/31 13:44:31 UTC
find bugs code check
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 !
Re: find bugs code check
Posted by Nikolai Kulagin <zz...@gmail.com>.
Hi. Igniters!
I create two PR [1,2] for two issues: IGNITE-9165 (close streams) [3] and
IGNITE-9160 (fix equals methods) [4]. Can you please review it? The changes
are quite small, tests are ok, flacky as usual.
[1] https://github.com/apache/ignite/pull/4481
[2] https://github.com/apache/ignite/pull/4478
[3] https://jira.apache.org/jira/browse/IGNITE-9165
[4] https://jira.apache.org/jira/browse/IGNITE-9160
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Re: find bugs code check
Posted by Nikolai Kulagin <zz...@gmail.com>.
Hi, igniters,
As a result of this discussion [1] two issue were created:
- NPE and CCE on equals() methods [2];
- Methods may fail to close stream in core module [3].
In addition to the list of bugs from the letter, FindBugs plug-in was
additionally analyzed the core module for the presence of similar problems.
Found bugs added to issues.
In the future, it is planned to expand the list, so the issues were tagged
with the FindBugs tag for easy navigation. It is also necessary to check the
remaining modules.
[1]
http://apache-ignite-developers.2346864.n4.nabble.com/find-bugs-code-check-td33261.html
[2] https://issues.apache.org/jira/projects/IGNITE/issues/IGNITE-9160
[3] https://issues.apache.org/jira/projects/IGNITE/issues/IGNITE-9165
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
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
>
Re: find bugs code check
Posted by 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