Ticket #6811 (closed: fixed)
MonitorLiveData(Test) race conditions
| Reported by: | Russell Taylor | Owned by: | Russell Taylor |
|---|---|---|---|
| Priority: | minor | Milestone: | Release 2.5 |
| Component: | Mantid | Keywords: | LiveData |
| Cc: | Blocked By: | ||
| Blocking: | Tester: | Martyn Gigg |
Description
I've noticed failures of this test on a relatively regular basis. Seemed like thread safety was the most likely culprit. So I put it through helgrind and the things that located will be fixed here....
Change History
comment:2 Changed 8 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:3 Changed 8 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:4 Changed 8 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:5 Changed 8 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:6 Changed 8 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:7 Changed 8 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:8 Changed 8 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:9 Changed 8 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:10 Changed 8 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:11 Changed 8 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:12 Changed 8 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:13 Changed 8 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:14 Changed 8 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:15 Changed 8 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:16 Changed 8 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:17 Changed 8 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:18 Changed 8 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:19 Changed 8 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:20 Changed 8 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:21 Changed 8 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:22 Changed 8 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:23 Changed 8 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:24 Changed 8 years ago by Russell Taylor
- Status changed from accepted to verify
- Resolution set to fixed
Tester: Branch is feature/6811_monitorlivedata_races
The work under this ticket expanded from the title into a general tidy-up of the AlgorithmManager class (in particular removing the algorithms() method therein). The commits [ecc5e98d] & [d95be53f] are the ones that actually address the thread-safety problems highlighted by helgrind.
Testing is mainly inspecting the code and checking that all the tests are behaving. You could run helgrind if you really wanted to.....
One particular thing to check is that algorithm cancellation from MantidPlot behaves - this ticket messed with that a bit.
comment:25 Changed 8 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:26 Changed 8 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:27 Changed 8 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:28 Changed 8 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:29 Changed 8 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:30 Changed 8 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:31 Changed 8 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:32 Changed 8 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:33 Changed 8 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:34 Changed 8 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:35 Changed 8 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:36 Changed 8 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:37 Changed 8 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:38 Changed 8 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:39 Changed 8 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:40 Changed 8 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:41 Changed 8 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:42 Changed 8 years ago by Martyn Gigg
- Status changed from verifying to closed
The code changes look good (hurrah for getting rid of algorithms()). Checked StartLiveData with the fake, file & instrument listeners and everything looked okay. The cancellation worked in all modes.
comment:43 Changed 7 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:44 Changed 7 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:45 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:46 Changed 7 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:47 Changed 7 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:48 Changed 7 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:49 Changed 7 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:50 Changed 7 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:51 Changed 7 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:52 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:53 Changed 7 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:54 Changed 7 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:55 Changed 7 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:56 Changed 7 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:57 Changed 7 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:58 Changed 7 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:59 Changed 7 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:60 Changed 7 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:61 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:62 Changed 7 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:63 Changed 7 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:64 Changed 7 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:65 Changed 7 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:66 Changed 7 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:67 Changed 7 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:68 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:69 Changed 7 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:70 Changed 7 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:71 Changed 7 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:72 Changed 7 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:73 Changed 7 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:74 Changed 7 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:75 Changed 7 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:76 Changed 7 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:77 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:78 Changed 7 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:79 Changed 7 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:80 Changed 7 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:81 Changed 7 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:82 Changed 7 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:83 Changed 7 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:84 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:85 Changed 7 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:86 Changed 7 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:87 Changed 7 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:88 Changed 7 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:89 Changed 7 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:90 Changed 7 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:91 Changed 7 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:92 Changed 7 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:93 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:94 Changed 7 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:95 Changed 7 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:96 Changed 7 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:97 Changed 7 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:98 Changed 7 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:99 Changed 7 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:100 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:101 Changed 7 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:102 Changed 7 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:103 Changed 7 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:104 Changed 7 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:105 Changed 7 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:106 Changed 7 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:107 Changed 7 years ago by Russell Taylor
Re #6811. Remove method that breaks encapsulation.
This breaks stuff. Fixing it is the next few steps....
Changeset: 8231b0c9446d20be2cb8fc474508d8e866569d05
comment:108 Changed 7 years ago by Russell Taylor
Re #6811. Add a method that returns all running algorithms
...of a given name. N.B. Had to temporarily comment out some tests that used the removed algorithms() method.
Changeset: 61cfbc33e8a4f76fe862c8da538f33522aa43bd3
comment:109 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's new runningInstancesOf method.
This was the place where there was previously a race condition. The iterators in the list returned by the algorithms() method could be invalidated by another thread creating more algorithms.
Changeset: ecc5e98dcb618198ec101ac32e0c36b6b0e360d5
comment:110 Changed 7 years ago by Russell Taylor
Re #6811. Re-work & re-enable tests that used algorithms() method.
We can do essentially the same testing via other public methods.
Changeset: 71cfd3a9624f39faec2ceff21a0ec4ec9dbe4bff
comment:111 Changed 7 years ago by Russell Taylor
Re #6811. Refactor test to avoid asynchronous algorithm execution.
I found a better way to mimic a long running algorithm rather than actually having one - just have it always say it's still running if asked. This is just as good for testing this class as the old way.
Changeset: bdcc4a14938255afb40e07acb4c9381eeacf2402
comment:112 Changed 7 years ago by Russell Taylor
Re #6811. Simplify & re-enable previously flaky test.
There's no longer any multi-threaded execution going on, so I think this test will behave now.
Changeset: bd266a4cfeae57af2fca1f43cfd53d77741cf803
comment:113 Changed 7 years ago by Russell Taylor
Re #6811. Add a cancelAll() method to AlgorithmManager.
Changeset: 802dbf7ac5a5c77481544a34e04b39a5282f9dcd
comment:114 Changed 7 years ago by Russell Taylor
Re #6811. Forward call to AlgorithmManager's cancelAll method.
This is better than pulling out the algorithm list and manipulating things up the top.
Changeset: 9ae37a9b15da67d50b23377964b647ce524d4fca
comment:115 Changed 7 years ago by Russell Taylor
Re #6811. Add method to return newest instance of an algorithm.
Changeset: 535776510f9a891b14a069db234fa9922e3c1b91
comment:116 Changed 7 years ago by Russell Taylor
Re #6811. Use AlgorithmManager's newestInstanceOf method.
In place of pulling out the list via the (now gone) algorithms() method and going through it up at this level. There is a slight difference in behaviour, in that the old way returned the newest algorithm that HADN'T been executed already. However, since the dialog is created immediately after the algorithm has been, I don't see how that can be a problem.
Changeset: 15670e8d8b10667bc9001043179f4b9d01e4acd5
comment:117 Changed 7 years ago by Russell Taylor
Re #6811. Remove unused (apart from in the test) method.
Changeset: d6f2b4318ad5006d9f49c096a39fe20b12f2909d
comment:118 Changed 7 years ago by Russell Taylor
Re #6811. Minor tidy-ups.
Changeset: 62bd36870470d3a086b11ea5d526a12aaf83245c
comment:119 Changed 7 years ago by Russell Taylor
Re #6811. Remove isRunningAsync() method - it is not used anywhere.
Changeset: 067610018b910416ff5a7508e9c17d3da9c42141
comment:120 Changed 7 years ago by Russell Taylor
Re #6811. Thread-safety around the running & cancel flags.
Also some const-correctness.
Changeset: d95be53f3710471bf68cabcb62d889774d5539ee
comment:121 Changed 7 years ago by Russell Taylor
Re #6811. Const-correctness.
Changeset: 5b60bdedd18cf2ba07d31d6ed09821d85cf03902
comment:122 Changed 7 years ago by Russell Taylor
Re #6811. Missed an override.
Changeset: 2b051b1eb0427f0e46e47dcebdd4ef3233022f5e
comment:123 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 7657
