[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user mharmer commented on the issue: https://github.com/apache/ant/pull/81 @jaikiran: "Mark Harmer" will work, thanks. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user jaikiran commented on the issue: https://github.com/apache/ant/pull/81 @mharmer, what name would you like us to add to our contributors list (https://github.com/apache/ant/blob/master/CONTRIBUTORS) for your contribution? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user jaikiran commented on the issue: https://github.com/apache/ant/pull/81 > @jaikiran I think the test does exhibit the multithreaded scenario, I ran it prior to the Project changes to ensure it did fail. My Java is a bit rusty, but I believe the 2 separate ExecutorService's created run concurrently. You are right indeed. When I replied previously, I did not focus on the second executor submit and the fact that this task would run in parallel with the previous for loop that (might) still be in progress. > My only concern is it's possible it may not catch the issue in the future as a regression test due to the non-determinism, as well as the somewhat arbitrary loop count and extended time taken to run the test (<100 milliseconds currently, but overall these types of tests tend to add up). Agreed. So it's fine to not include the test. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user mharmer commented on the issue: https://github.com/apache/ant/pull/81 @jaikiran I think the test does exhibit the multithreaded scenario, I ran it prior to the `Project` changes to ensure it did fail. My Java is a bit rusty, but I believe the 2 separate `ExecutorService`'s created run concurrently. My only concern is it's possible it may not catch the issue in the future as a regression test due to the non-determinism, as well as the somewhat arbitrary loop count and extended time taken to run the test (<100 milliseconds currently, but overall these types of tests tend to add up). --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user asfgit commented on the issue: https://github.com/apache/ant/pull/81 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/Ant%20Github-PR-Windows/102/ --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user asfgit commented on the issue: https://github.com/apache/ant/pull/81 Refer to this link for build results (access rights to CI server needed): https://builds.apache.org/job/Ant%20Github-PR-Linux/96/ --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user jaikiran commented on the issue: https://github.com/apache/ant/pull/81 I don't have enough knowledge about the thread safety aspects of the `Project` class. Stefan (@bodewig) would know more. However, having look at its code, I see similar locks and thread safety constructs for other members, so I think this change is fine. As for the testcase, I think it's OK to add one. However, the snippet that you pasted might need some work. Right now, it uses a single thread and then in that thread's execution runs the APIs of the `Project` class in a loop. So it really doesn't exercise the multi-thread access behaviour and the access will all be sequential. What you could do is, use a fixed thread pool with a decent size (10 perhaps) and then use a loop (of 10) to submit the `p.CopyOfReferences` in that loop and finally `get()` the result for each of those `Future`s. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user jaikiran commented on the issue: https://github.com/apache/ant/pull/81 this is ok to test --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user mharmer commented on the issue:
https://github.com/apache/ant/pull/81
I have a test case I wrote as well, however I was hesitant on adding it
since it dealt with non-deterministic behavior of trying to reproduce the race
condition. If it's desired I can add it, it's basically this:
```
/**
* This test ensures that when requesting Project.getCopyOfReferences a
* ConcurrentModificationException isn't thrown.
*
* ExecutorService is used so Exceptions are rethrown from their
executors
* and caught by JUnit as an error.
*/
@Test
public void testMultithreadedGetReferences() throws Exception {
final ExecutorService es1 = Executors.newSingleThreadExecutor();
final Future getReferencesThread = es1.submit(() -> {
for (int i = 0; i < 1000; i++) {
p.getCopyOfReferences();
}
});
final ExecutorService es2 = Executors.newSingleThreadExecutor();
final Future addReferencesThread = es2.submit(() -> {
for (int i = 0; i < 1000; i++) {
p.addReference("dummy" + i, "dummyValue");
}
});
getReferencesThread.get();
addReferencesThread.get();
}
```
---
-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user asfgit commented on the issue: https://github.com/apache/ant/pull/81 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant issue #81: Fix rare ConcurrentModificationException when running with Pa...
Github user asfgit commented on the issue: https://github.com/apache/ant/pull/81 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
