Re: RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-13 Thread Daniel Fuchs
Hi Pavel, On 13/03/2019 16:35, Pavel Rappo wrote: Using a bunch of Semaphores here seems a bit surprising. One more CountDownLatch or CyclicBarrier *might* have been better. But I guess it's not of concern. Agreed, but I won't change it. The reason is that I had a more extensive version of

Re: RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-13 Thread Pavel Rappo
Using a bunch of Semaphores here seems a bit surprising. One more CountDownLatch or CyclicBarrier *might* have been better. But I guess it's not of concern. For the diagnostic purposes I would add a newline between t4.join(); t3.join(); since they throw exceptions. Otherwise looks good.

Re: RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-13 Thread Daniel Fuchs
Hi Pavel, On 13/03/2019 14:41, Pavel Rappo wrote: After all if this group is being destroyed, and this is the only call, then 0 would be a correct value, wouldn't it? It may only be the correct value if the input argument value n == 0. As the @summary section says: the 3-args enumerate method

Re: RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-13 Thread Pavel Rappo
> On 13 Mar 2019, at 14:22, Daniel Fuchs wrote: > > I don't think there is another bug in the 3-args enumerate > methods. I might not have expressed myself clearly. I'm not saying there is an extra bug with the fix applied. What I'm saying is that without the fix, not only was the value wrong

Re: RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-13 Thread Daniel Fuchs
Hi Pavel, On 13/03/2019 14:02, Pavel Rappo wrote: Might it be the case that not only the returned value is wrong, but also the populated list is corrupted? After all the returned value is used as an index for the next insertion. I'm not sure what you mean by that. I don't think there is

Re: RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-13 Thread Pavel Rappo
Hi Daniel, First of all, the change looks good to me. The broken if (destroyed) { return n; } part looks like a "copy and paste" from some other method in the ThreadGroup class. They use recursion a lot. Might it be the case that not only the returned value is wrong, but also the

RFR: 8219197: ThreadGroup.enumerate() may return wrong value

2019-03-12 Thread Daniel Fuchs
Hi, Please find below a simple fix for 8219197: ThreadGroup.enumerate() may return wrong value http://cr.openjdk.java.net/~dfuchs/webrev_8219197/webrev.00/ This is a bug in the implementation of the recursion, as enumerate(list, n, recurse) should never have returned a value < n. The test