You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by jdanekrh <gi...@git.apache.org> on 2018/10/16 10:47:48 UTC
[GitHub] qpid-proton-j pull request #19: PROTON-1958 Fix use of AtomicInteger for ins...
GitHub user jdanekrh opened a pull request:
https://github.com/apache/qpid-proton-j/pull/19
PROTON-1958 Fix use of AtomicInteger for instance IDs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jdanekrh/qpid-proton-j jd_staticatomic
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/qpid-proton-j/pull/19.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19
----
commit e9724b7d26644e8f78fbccc356ab748e295801cd
Author: Jiri Danek <jd...@...>
Date: 2018-10-15T09:30:43Z
PROTON-1958 Fix use of AtomicInteger for instance IDs
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/qpid-proton-j/pull/19
I'll merge this later, could perhaps do with a test.
Note that the example change should really have been kept separate as it has nothing to do with the defect (and as noted before, wont really make any difference to the example since it will typically only have 1 instance)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/qpid-proton-j/pull/19
Can you please close out this PR? I went with a different approach, committed in 83609c4752fbdaa1ddfa032285e5caa09b61f480, of making each reactor timer maintain a counter for its tasks, rather than a shared counter across the JVM for all reactors.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...
Posted by jdanekrh <gi...@git.apache.org>.
Github user jdanekrh commented on the issue:
https://github.com/apache/qpid-proton-j/pull/19
Thanks. I took away the example change.
About the test. What happens is that PriorityQueue dequeues equal items in arbitrary order. So the potential user-facing problem is that user creates two tasks with the intention of first running first, which would not happen due to the bug.
My only idea for deterministic test would be something like
```
public class TaskImplTest {
@Test
public void testCompareTo_greaterDeadlineObjectIsGreater() {
TaskImpl t42 = new TaskImpl(42);
TaskImpl t24 = new TaskImpl(24);
Assert.assertTrue(t24.compareTo(t42) <= 0);
}
@Test
public void testCompareTo_equalDeadlineLaterCreatedObjectIsGreater() {
TaskImpl t42a = new TaskImpl(42);
TaskImpl t42b = new TaskImpl(42);
Assert.assertTrue(t42a.compareTo(t42b) <= 0);
}
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] qpid-proton-j pull request #19: PROTON-1958 Fix use of AtomicInteger for ins...
Posted by jdanekrh <gi...@git.apache.org>.
Github user jdanekrh closed the pull request at:
https://github.com/apache/qpid-proton-j/pull/19
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org