On 7/25/17 1:15 AM, Mandy Chung wrote:
Webrev updated:http://cr.openjdk.java.net/~amlu/8183377/webrev.01/
Looks good. One minor suggestion is to place the source files under a
directory showing its package hierachy e.g. src/comSA/Alice.java,
src/comSB/Bob.java
No need for a new webrev.
Than
> On Jul 17, 2017, at 12:49 AM, Amy Lu wrote:
>
> On 7/14/17 4:41 PM, Mandy Chung wrote:
>> I think compiling the other classes to a destination explicitly to replace:
>> 29 * @build Alice Bob SupAlice SupBob
>> 30 * @run driver SetupLoader
>>
>> will make the test clearer. It’d be good
On 7/14/17 4:41 PM, Mandy Chung wrote:
I think compiling the other classes to a destination explicitly to replace:
29 * @build Alice Bob SupAlice SupBob
30 * @run driver SetupLoader
will make the test clearer. It’d be good to make that change.
Webrev updated: http://cr.openjdk.java.ne
> On Jul 14, 2017, at 4:26 PM, Weijun Wang wrote:
>
>
>> On Jul 14, 2017, at 4:23 PM, Mandy Chung wrote:
>>
>>
>>> On Jul 13, 2017, at 5:14 PM, Amy Lu wrote:
>>>
>>> java/lang/ClassLoader/deadlock/TestCrossDelegate.sh
&g
> On Jul 14, 2017, at 4:23 PM, Mandy Chung wrote:
>
>
>> On Jul 13, 2017, at 5:14 PM, Amy Lu wrote:
>>
>> java/lang/ClassLoader/deadlock/TestCrossDelegate.sh
>> java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh
>>
>> Please review t
> On Jul 13, 2017, at 5:14 PM, Amy Lu wrote:
>
> java/lang/ClassLoader/deadlock/TestCrossDelegate.sh
> java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh
>
> Please review this patch to refactor the shell tests to java.
>
> bug: https://bugs.openjdk.java.net/b
java/lang/ClassLoader/deadlock/TestCrossDelegate.sh
java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh
Please review this patch to refactor the shell tests to java.
bug: https://bugs.openjdk.java.net/browse/JDK-8183377
http://cr.openjdk.java.net/~amlu/8183377/webrev.00/
Thanks,
Amy
From: core-libs-dev on behalf of Brent
Christian
Sent: Tuesday, May 17, 2016 8:46 PM
To: Mandy Chung; David Holmes
Cc: core-libs-dev
Subject: Re: RFR 8029891 : Deadlock detected in
java/lang/ClassLoader/deadlock/GetResource.java - A Revival
Hi!
Here's the
Hi Brent,
The unsynchronized test looks good.
Let's hope that we didn't miss any hidden detail. We'll see how this
works out when people get hold of new build containing this change.
Regards, Peter
On 05/18/2016 03:46 AM, Brent Christian wrote:
Hi!
Here's the final(?) webrev:
http://cr.
On 18/05/2016 11:46 AM, Brent Christian wrote:
Hi!
Here's the final(?) webrev:
http://cr.openjdk.java.net/~bchristi/8029891/webrev.6/webrev/
with the following changes since the last one:
No further comments from me.
Thanks,
David
* The @hidden JavaDoc tag has been removed. It can't be u
> On May 17, 2016, at 6:46 PM, Brent Christian
> wrote:
>
> Hi!
>
> Here's the final(?) webrev:
> http://cr.openjdk.java.net/~bchristi/8029891/webrev.6/webrev/
+1
Nit: line 132, 134 in PropertiesSerialization.java test are long lines that
should be wrapped to next line.
Mandy
Hi!
Here's the final(?) webrev:
http://cr.openjdk.java.net/~bchristi/8029891/webrev.6/webrev/
with the following changes since the last one:
* The @hidden JavaDoc tag has been removed. It can't be used due to
methods disappearing from the API page. Living with the additional
JavaDoc is the
> On May 12, 2016, at 5:58 PM, David Holmes wrote:
>>
>> With all of the inherited methods @hidden, it looks like that section
>> is left out altogether.
>
> Okay, so I have to say @hidden seems to me to be seriously flawed! If a class
> has a method that can be called then IMHO that has to be
> On May 12, 2016, at 4:55 PM, Brent Christian
> wrote:
>
> Update to the test, and additional comments:
>
> http://cr.openjdk.java.net/~bchristi/8029891/webrev.5/webrev/
>
+1
Mandy
Good idea.
Mandy
> On May 13, 2016, at 12:10 AM, Peter Levart wrote:
>
> Hi Brent,
>
>
> This looks good. It might also be a good idea to add a test that checks this
> new implementation detail (the unsynchronized get) that new code will become
> dependent upon, for example:
>
>
> /*
> *
Note that there is a possible initialization circularity introduced by
this patch, which I think is harmless because:
- ThreadLocalRandom has just recently been enhanced to cope with such
situations - CHM needs ThreadLocalRandom in put() for example, when new
entry is being inserted, TLR invok
Hi Brent,
This looks good. It might also be a good idea to add a test that checks
this new implementation detail (the unsynchronized get) that new code
will become dependent upon, for example:
/*
* @test
* @bug 8029891
* @summary Test that the Properties.get() method does not synchronize
Hi Brent,
On 13/05/2016 7:02 AM, Brent Christian wrote:
Hi, David
On 5/11/16 8:39 PM, David Holmes wrote:
On 11/05/2016 7:56 AM, Brent Christian wrote:
While good progress was made during the original code review, all of the
overridden methods in Properties caused an explosion of unnecessary
Update to the test, and additional comments:
http://cr.openjdk.java.net/~bchristi/8029891/webrev.5/webrev/
Thanks,
-Brent
On 5/12/16 4:15 PM, Mandy Chung wrote:
On May 12, 2016, at 2:44 PM, Brent Christian wrote:
On 5/12/16 11:44 AM, Mandy Chung wrote:
This patch looks good.
To help fut
> On May 12, 2016, at 2:44 PM, Brent Christian
> wrote:
>
> On 5/12/16 11:44 AM, Mandy Chung wrote:
>>
>> This patch looks good.
>>
>> To help future readers to understand this, it may be better to move:
>> 1152 private transient ConcurrentHashMap map =
>> 1153 new ConcurrentH
On 5/12/16 11:44 AM, Mandy Chung wrote:
This patch looks good.
To help future readers to understand this, it may be better to move:
1152 private transient ConcurrentHashMap map =
1153 new ConcurrentHashMap<>(8);
to the beginning and add a comment describing what are lock-free a
Hi, David
On 5/11/16 8:39 PM, David Holmes wrote:
On 11/05/2016 7:56 AM, Brent Christian wrote:
While good progress was made during the original code review, all of the
overridden methods in Properties caused an explosion of unnecessary
JavaDoc (see specdiff at [2]). With the recent fix of 807
> On May 11, 2016, at 8:39 PM, David Holmes wrote:
>
>> While good progress was made during the original code review, all of the
>> overridden methods in Properties caused an explosion of unnecessary
>> JavaDoc (see specdiff at [2]). With the recent fix of 8073100 (new
>> "@hidden" JavaDoc tag)
Hi Brent,
>
> A new webrev is here:
> http://cr.openjdk.java.net/~bchristi/8029891/webrev.4/
This patch looks good.
To help future readers to understand this, it may be better to move:
1152 private transient ConcurrentHashMap map =
1153 new ConcurrentHashMap<>(8);
to the beginn
Hi Brent,
On 11/05/2016 7:56 AM, Brent Christian wrote:
Hi! Welcome back to 8029891, "Deadlock detected in
java/lang/ClassLoader/deadlock/GetResource.java". Our previous
discussion is at [1].
Briefly, java.util.Properties isa Hashtable, which synchronizes on
itself for
On 5/10/16 2:56 PM, Brent Christian wrote:
While good progress was made during the original code review, all of the
overridden methods in Properties caused an explosion of unnecessary
JavaDoc (see specdiff at [2]). With the recent fix of 8073100 (new
"@hidden" JavaDoc tag), we can now avoid the
Hi! Welcome back to 8029891, "Deadlock detected in
java/lang/ClassLoader/deadlock/GetResource.java". Our previous
discussion is at [1].
Briefly, java.util.Properties isa Hashtable, which synchronizes on
itself for any access. System.getProperties() returns the Properties
instanc
The 0x040ebee8 monitor is most likely being held by native code.
Regards,
Peter Firmstone.
On 7/02/2016 4:27 AM, Peter Levart wrote:
On 02/06/2016 01:17 PM, Peter Firmstone wrote:
The security manager is non blocking, unfortunately java system
classes aren't.
It doesn't seem so. At least
Thanks Peter & David,
That's good to know.
I've altered the SecurityManager to perform a permission check prior to
becoming the security manager, this ensures the cache has been created,
to avoid any recursive deadlock prone calls.
The class that both threads are attempting to load and init
Out of curiousity, what class loader is using to define
org.apache.river.api.security.CombinerSecurityManager?
Mandy
> On Feb 7, 2016, at 1:54 AM, Peter wrote:
>
> Thanks Peter & David,
>
> That's good to know.
>
> I've altered the SecurityManager to perform a permission check prior to
> be
Thanks Peter & David,
That's good to know.
I've altered the SecurityManager to perform a permission check prior to
becoming the security manager, this ensures the cache has been created,
to avoid any recursive deadlock prone calls.
The class that both threads are attempting to load and init is
On 02/06/2016 10:32 PM, Peter wrote:
The 0x040ebee8 monitor is most likely being held by native code.
Regards,
Peter Firmstone.
Ok, but where? The deadlock report says it is held by main thread.
Somewhere between the following two java frames?
"main" #1 prio=5 os_prio=0 tid=0x017cf400 ni
On 7/02/2016 4:27 AM, Peter Levart wrote:
On 02/06/2016 01:17 PM, Peter Firmstone wrote:
The security manager is non blocking, unfortunately java system
classes aren't.
It doesn't seem so. At least, I can't find where main thread locks the
0x040ebee8 monitor:
"Thread-1":
waiting to lock
On 6/02/2016 10:17 PM, Peter Firmstone wrote:
The security manager is non blocking, unfortunately java system classes
aren't.
The security manager has a lot of dependencies on other code/classes and
so requires lots of additional classloading to execute as desired. When
the SM is itself used
On 02/06/2016 01:17 PM, Peter Firmstone wrote:
The security manager is non blocking, unfortunately java system
classes aren't.
It doesn't seem so. At least, I can't find where main thread locks the
0x040ebee8 monitor:
"Thread-1":
waiting to lock monitor 0x142766ac (object 0x040ebee8, a [
The security manager is non blocking, unfortunately java system classes
aren't.
The policy provider in use thread confines PermissionCollection
instances, which never leave the cpu cache, they are discarded
immediately after use.
The problem here is that two different threads are attempting
On 6/02/2016 9:39 PM, Peter Firmstone wrote:
Hmm, thought the new parallel lock stategy in ClassLoader wasn't
deadlock prone? Guess I was wrong.
The deadlock is introduced by a custom security manager. SM's play a
critical role in many parts of the core libraries so if they utilize
synchroni
Hmm, thought the new parallel lock stategy in ClassLoader wasn't
deadlock prone? Guess I was wrong.
Observation: On an unrelated occassion, I had a URLClassLoader
synchronization hotspot (well not standard URLClassLoader, but a high
performance RFC3986 URL ClassLoader, that use normalized RF
> On May 15, 2015, at 12:09 PM, Brent Christian
> wrote:
>
> Updated webrev:
>
> http://cr.openjdk.java.net/~bchristi/8029891/webrev.1/
>
> I have restored synchronization to modification methods, and to my
> interpretation of "bulk read" operations:
>
>* forEach
>* replaceAll
>
Hi, Daniel
You're right, thanks.
The situation is the same for elements(). I've updated these in my
local repo.
I checked through the methods that return a Set/Enumeration/etc, and I
believe there's also an issue with entrySet(). The returned Set should
be backed by the map, and support r
On 16/05/2015 5:09 AM, Brent Christian wrote:
On 5/13/15 8:53 AM, Mandy Chung wrote:
On May 12, 2015, at 2:26 PM, Peter Levart
wrote:
On 05/12/2015 10:49 PM, Mandy Chung wrote:
But I think it should be pretty safe to make the
java.util.Properties object override all Hashtable methods and
deleg
Hi Brent,
1163 @Override
1164 public Enumeration keys() {
1165 return map.keys();
1166 }
I wonder if you should use:
public Enumeration keys() {
return Collections.enumeration(map.keySet());
}
instead.
ConcurrentHashMap.keys() returns an Enumeration which is also an
It
On 5/13/15 8:53 AM, Mandy Chung wrote:
On May 12, 2015, at 2:26 PM, Peter Levart
wrote:
On 05/12/2015 10:49 PM, Mandy Chung wrote:
But I think it should be pretty safe to make the
java.util.Properties object override all Hashtable methods and
delegate to internal CMH so that:
- all modification
On 05/13/2015 01:45 AM, Paul Sandoz wrote:
On May 12, 2015, at 10:49 PM, Mandy Chung wrote:
How likely does existing code do this? A proper way is to call
System.setProperty. One pattern I found on System.setProperties is like this
to add a system property of key/value pair:
Properties p
> On May 12, 2015, at 2:26 PM, Peter Levart wrote:
>
>
>
> On 05/12/2015 10:49 PM, Mandy Chung wrote:
>>> But I think it should be pretty safe to make the java.util.Properties
>>> object override all Hashtable methods and delegate to internal CMH so that:
>>> - all modification methods + all
On May 12, 2015, at 10:49 PM, Mandy Chung wrote:
>>
>> Ah, I understand Mandy now. You are talking about using special Properties
>> implementation just for system properties. Unfortunately, this is currently
>> valid code:
>>
>> Properties props = new Properties();
>> ...
>> System.setPropert
On 05/12/2015 10:49 PM, Mandy Chung wrote:
But I think it should be pretty safe to make the java.util.Properties
object override all Hashtable methods and delegate to internal CMH so
that:
- all modification methods + all bulk read methods such as
(keySet().toArray, values.toArray) are made s
On 05/11/2015 11:41 PM, Peter Levart wrote:
On 05/12/2015 07:41 AM, Peter Levart wrote:
Taking another look at this deadlock issue and the compatibility
concerns, I wonder if we should keep this change as a special
implementation for system properties rather than having this change to
java.u
On 05/12/2015 07:41 AM, Peter Levart wrote:
Taking another look at this deadlock issue and the compatibility
concerns, I wonder if we should keep this change as a special
implementation for system properties rather than having this change to
java.util.Properties class. Properties is a Hashtabl
On 05/12/2015 01:43 AM, Brent Christian wrote:
Hi, Mandy. Thanks for having a look.
On 5/11/15 12:09 PM, Mandy Chung wrote:
~/WORK/8029891/plevartI'd like Daniel's suggestion to have the load*
method to putAll entries in one go after the input reader/stream/xml
file is loaded and parsed rath
> On May 11, 2015, at 4:43 PM, Brent Christian
> wrote:
>
> Hi, Mandy. Thanks for having a look.
>
> On 5/11/15 12:09 PM, Mandy Chung wrote:
>> ~/WORK/8029891/plevartI'd like Daniel's suggestion to have the load*
>> method to putAll entries in one go after the input reader/stream/xml
>> file
Hi, Mandy. Thanks for having a look.
On 5/11/15 12:09 PM, Mandy Chung wrote:
~/WORK/8029891/plevartI'd like Daniel's suggestion to have the load*
method to putAll entries in one go after the input reader/stream/xml
file is loaded and parsed rather than adding one entry at a time.
I also like
Hi Brent,
On 05/04/2015 09:11 AM, Brent Christian wrote:
Hi,
Please review this fix, courtesy of Peter Levart (thanks!), that I
would like to get in.
https://bugs.openjdk.java.net/browse/JDK-8029891
http://cr.openjdk.java.net/~bchristi/8029891/webrev.0/
Thanks for taking this on. I haven'
On 06/05/15 11:41, David Holmes wrote:
I don't think you want to de-synchronize the load* methods - you don't
want two threads calling load concurrently. But that then raises the
problem of concurrent modification while a load is in progress.
Synchronization ensures serialization and by removing
On 6/05/2015 5:57 PM, Peter Levart wrote:
On 05/05/2015 03:25 AM, David Holmes wrote:
Hi Brent,
On 5/05/2015 2:11 AM, Brent Christian wrote:
Hi,
Please review this fix, courtesy of Peter Levart (thanks!), that I would
like to get in.
https://bugs.openjdk.java.net/browse/JDK-8029891
http://
On 05/05/2015 03:25 AM, David Holmes wrote:
Hi Brent,
On 5/05/2015 2:11 AM, Brent Christian wrote:
Hi,
Please review this fix, courtesy of Peter Levart (thanks!), that I would
like to get in.
https://bugs.openjdk.java.net/browse/JDK-8029891
http://cr.openjdk.java.net/~bchristi/8029891/webre
Hi Brent,
On 5/05/2015 2:11 AM, Brent Christian wrote:
Hi,
Please review this fix, courtesy of Peter Levart (thanks!), that I would
like to get in.
https://bugs.openjdk.java.net/browse/JDK-8029891
http://cr.openjdk.java.net/~bchristi/8029891/webrev.0/
There is some discussion of it in the bug
Hi Brent,
I think all you need for test is this:
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;
public class CheckOverrides {
public static void main(String[] args) {
Set p
On 05/04/2015 06:11 PM, Brent Christian wrote:
Hi,
Please review this fix, courtesy of Peter Levart (thanks!), that I
would like to get in.
https://bugs.openjdk.java.net/browse/JDK-8029891
http://cr.openjdk.java.net/~bchristi/8029891/webrev.0/
There is some discussion of it in the bug repo
Hi,
Please review this fix, courtesy of Peter Levart (thanks!), that I would
like to get in.
https://bugs.openjdk.java.net/browse/JDK-8029891
http://cr.openjdk.java.net/~bchristi/8029891/webrev.0/
There is some discussion of it in the bug report, starting at 2014-12-31.
The problem, as state
Thanks, Chris.
On 12/30/14 12:52 AM, Chris Hegarty wrote:
I’m ok with this Brent.
-Chris.
On 29 Dec 2014, at 21:43, Brent Christian wrote:
Hi,
The
java/lang/ClassLoader/deadlock/GetResource.java
test has started causing problems for Hotspot nightly testing. A "real" fix
I’m ok with this Brent.
-Chris.
On 29 Dec 2014, at 21:43, Brent Christian wrote:
> Hi,
>
> The
> java/lang/ClassLoader/deadlock/GetResource.java
> test has started causing problems for Hotspot nightly testing. A "real" fix
> is being worked on under [1], bu
Hi,
The
java/lang/ClassLoader/deadlock/GetResource.java
test has started causing problems for Hotspot nightly testing. A "real"
fix is being worked on under [1], but in the meantime, this test should
be added to the ProblemList.
Bug:
https://bugs.openjdk.java.net/browse/JDK-80
Changeset: 70f14c2db078
Author:alanb
Date: 2011-06-21 16:11 +0100
URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/70f14c2db078
7056815: test/java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh times out
intermittently on busy machine
Reviewed-by: mchung
! test/java/lang
On 6/20/11 9:15 PM, Alan Bateman wrote:
There is another bug asking that the ClassLoader tests be updated to
work with cygwin:
7036526: TEST_BUG: add cygwin support to tests in
test/closed/java/lang/ClassLoader
I don't really have time at the moment to fix these tests and the only
reason I
Mandy Chung wrote:
Alan,
The fix to remove the timeout value looks okay to me. While you're
there, I wonder if the test passes on Windows with Cygwin and looks
like this test might have a similar bug:
6981005: TEST BUG: java/lang/ClassLoader/TestCrossDelegate.sh
timeout on windows
that
tests concurrently. The problem
is the specified timeout (10s) is too low unless scaled with the
-timeoutFactor option. I don't see any any reason for this test to
override the default timeout so the following patch removes it.
-Alan
diff --git a/test/java/lang/ClassLoader/deadlock/TestOneWayDele
fault timeout so the following patch removes it.
-Alan
diff --git a/test/java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh
b/test/java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh
--- a/test/java/lang/ClassLoader/deadlock/TestOneWayDelegate.sh
+++ b/test/java/lang/ClassLoader/dea
Changeset: 8329ebeabc10
Author:mchung
Date: 2010-08-16 15:36 -0700
URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/8329ebeabc10
6921234: TEST_BUG: java/lang/ClassLoader/deadlock/TestCrossDelegate.sh needs to
be modified for Cygwin
Summary: Add check for CYGWIN
Reviewed-by: ohair
69 matches
Mail list logo
