6963811: Deadlock-prone locking changes in Introspector
Reviewed-by: peterz, rupashka
--- a/jdk/src/share/classes/com/sun/beans/finder/InstanceFinder.java Fri Jul 09 19:42:17 2010 +0400
+++ b/jdk/src/share/classes/com/sun/beans/finder/InstanceFinder.java Fri Jul 09 22:07:39 2010 +0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -39,7 +39,7 @@
private final Class<? extends T> type;
private final boolean allow;
private final String suffix;
- private String[] packages;
+ private volatile String[] packages;
InstanceFinder(Class<? extends T> type, boolean allow, String suffix, String... packages) {
this.type = type;
@@ -49,9 +49,7 @@
}
public String[] getPackages() {
- return (this.packages.length > 0)
- ? this.packages.clone()
- : this.packages;
+ return this.packages.clone();
}
public void setPackages(String... packages) {
--- a/jdk/src/share/classes/com/sun/beans/finder/PersistenceDelegateFinder.java Fri Jul 09 19:42:17 2010 +0400
+++ b/jdk/src/share/classes/com/sun/beans/finder/PersistenceDelegateFinder.java Fri Jul 09 22:07:39 2010 +0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -47,17 +47,22 @@
}
public void register(Class<?> type, PersistenceDelegate delegate) {
- if (delegate != null) {
- this.registry.put(type, delegate);
- }
- else {
- this.registry.remove(type);
+ synchronized (this.registry) {
+ if (delegate != null) {
+ this.registry.put(type, delegate);
+ }
+ else {
+ this.registry.remove(type);
+ }
}
}
@Override
public PersistenceDelegate find(Class<?> type) {
- PersistenceDelegate delegate = this.registry.get(type);
+ PersistenceDelegate delegate;
+ synchronized (this.registry) {
+ delegate = this.registry.get(type);
+ }
return (delegate != null) ? delegate : super.find(type);
}
}
--- a/jdk/src/share/classes/com/sun/beans/finder/PropertyEditorFinder.java Fri Jul 09 19:42:17 2010 +0400
+++ b/jdk/src/share/classes/com/sun/beans/finder/PropertyEditorFinder.java Fri Jul 09 22:07:39 2010 +0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -64,12 +64,18 @@
}
public void register(Class<?> type, Class<?> editor) {
- this.registry.put(type, editor);
+ synchronized (this.registry) {
+ this.registry.put(type, editor);
+ }
}
@Override
public PropertyEditor find(Class<?> type) {
- PropertyEditor editor = instantiate(this.registry.get(type), null);
+ Class<?> predefined;
+ synchronized (this.registry) {
+ predefined = this.registry.get(type);
+ }
+ PropertyEditor editor = instantiate(predefined, null);
if (editor == null) {
editor = super.find(type);
if ((editor == null) && (null != type.getEnumConstants())) {
--- a/jdk/src/share/classes/java/beans/Encoder.java Fri Jul 09 19:42:17 2010 +0400
+++ b/jdk/src/share/classes/java/beans/Encoder.java Fri Jul 09 22:07:39 2010 +0400
@@ -194,13 +194,8 @@
* @see java.beans.BeanInfo#getBeanDescriptor
*/
public PersistenceDelegate getPersistenceDelegate(Class<?> type) {
- synchronized (this.finder) {
- PersistenceDelegate pd = this.finder.find(type);
- if (pd != null) {
- return pd;
- }
- }
- return MetaData.getPersistenceDelegate(type);
+ PersistenceDelegate pd = this.finder.find(type);
+ return (pd != null) ? pd : MetaData.getPersistenceDelegate(type);
}
/**
@@ -214,9 +209,7 @@
* @see java.beans.BeanInfo#getBeanDescriptor
*/
public void setPersistenceDelegate(Class<?> type, PersistenceDelegate delegate) {
- synchronized (this.finder) {
- this.finder.register(type, delegate);
- }
+ this.finder.register(type, delegate);
}
/**
--- a/jdk/src/share/classes/java/beans/Introspector.java Fri Jul 09 19:42:17 2010 +0400
+++ b/jdk/src/share/classes/java/beans/Introspector.java Fri Jul 09 22:07:39 2010 +0400
@@ -158,21 +158,23 @@
if (!ReflectUtil.isPackageAccessible(beanClass)) {
return (new Introspector(beanClass, null, USE_ALL_BEANINFO)).getBeanInfo();
}
+ Map<Class<?>, BeanInfo> beanInfoCache;
+ BeanInfo beanInfo;
synchronized (BEANINFO_CACHE) {
- Map<Class<?>, BeanInfo> beanInfoCache =
- (Map<Class<?>, BeanInfo>) AppContext.getAppContext().get(BEANINFO_CACHE);
-
+ beanInfoCache = (Map<Class<?>, BeanInfo>) AppContext.getAppContext().get(BEANINFO_CACHE);
if (beanInfoCache == null) {
beanInfoCache = new WeakHashMap<Class<?>, BeanInfo>();
AppContext.getAppContext().put(BEANINFO_CACHE, beanInfoCache);
}
- BeanInfo beanInfo = beanInfoCache.get(beanClass);
- if (beanInfo == null) {
- beanInfo = (new Introspector(beanClass, null, USE_ALL_BEANINFO)).getBeanInfo();
+ beanInfo = beanInfoCache.get(beanClass);
+ }
+ if (beanInfo == null) {
+ beanInfo = new Introspector(beanClass, null, USE_ALL_BEANINFO).getBeanInfo();
+ synchronized (BEANINFO_CACHE) {
beanInfoCache.put(beanClass, beanInfo);
}
- return beanInfo;
}
+ return beanInfo;
}
/**
@@ -302,10 +304,7 @@
*/
public static String[] getBeanInfoSearchPath() {
- BeanInfoFinder finder = getFinder();
- synchronized (finder) {
- return finder.getPackages();
- }
+ return getFinder().getPackages();
}
/**
@@ -329,10 +328,7 @@
if (sm != null) {
sm.checkPropertiesAccess();
}
- BeanInfoFinder finder = getFinder();
- synchronized (finder) {
- finder.setPackages(path);
- }
+ getFinder().setPackages(path);
}
@@ -454,10 +450,7 @@
* @return Instance of an explicit BeanInfo class or null if one isn't found.
*/
private static BeanInfo findExplicitBeanInfo(Class beanClass) {
- BeanInfoFinder finder = getFinder();
- synchronized (finder) {
- return finder.find(beanClass);
- }
+ return getFinder().find(beanClass);
}
/**
--- a/jdk/src/share/classes/java/beans/PropertyEditorManager.java Fri Jul 09 19:42:17 2010 +0400
+++ b/jdk/src/share/classes/java/beans/PropertyEditorManager.java Fri Jul 09 22:07:39 2010 +0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2010, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -81,10 +81,7 @@
if (sm != null) {
sm.checkPropertiesAccess();
}
- PropertyEditorFinder finder = getFinder();
- synchronized (finder) {
- finder.register(targetType, editorClass);
- }
+ getFinder().register(targetType, editorClass);
}
/**
@@ -95,10 +92,7 @@
* The result is null if no suitable editor can be found.
*/
public static PropertyEditor findEditor(Class<?> targetType) {
- PropertyEditorFinder finder = getFinder();
- synchronized (finder) {
- return finder.find(targetType);
- }
+ return getFinder().find(targetType);
}
/**
@@ -110,10 +104,7 @@
* e.g. Sun implementation initially sets to {"sun.beans.editors"}.
*/
public static String[] getEditorSearchPath() {
- PropertyEditorFinder finder = getFinder();
- synchronized (finder) {
- return finder.getPackages();
- }
+ return getFinder().getPackages();
}
/**
@@ -134,10 +125,7 @@
if (sm != null) {
sm.checkPropertiesAccess();
}
- PropertyEditorFinder finder = getFinder();
- synchronized (finder) {
- finder.setPackages(path);
- }
+ getFinder().setPackages(path);
}
private static PropertyEditorFinder getFinder() {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/beans/Introspector/Test6963811.java Fri Jul 09 22:07:39 2010 +0400
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 6963811
+ * @summary Tests deadlock in Introspector
+ * @author Sergey Malenkov
+ */
+
+import java.beans.Introspector;
+import java.beans.SimpleBeanInfo;
+
+public class Test6963811 implements Runnable {
+ private final long time;
+ private final boolean sync;
+
+ public Test6963811(long time, boolean sync) {
+ this.time = time;
+ this.sync = sync;
+ }
+
+ public void run() {
+ try {
+ Thread.sleep(this.time); // increase the chance of the deadlock
+ Introspector.getBeanInfo(
+ this.sync ? Super.class : Sub.class,
+ this.sync ? null : Object.class);
+ }
+ catch (Exception exception) {
+ exception.printStackTrace();
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ Thread[] threads = new Thread[2];
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(new Test6963811(0L, i > 0));
+ threads[i].start();
+ Thread.sleep(500L); // increase the chance of the deadlock
+ }
+ for (Thread thread : threads) {
+ thread.join();
+ }
+ }
+
+ public static class Super {
+ }
+
+ public static class Sub extends Super {
+ }
+
+ public static class SubBeanInfo extends SimpleBeanInfo {
+ public SubBeanInfo() {
+ new Test6963811(1000L, true).run();
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/beans/PropertyEditor/Test6963811.java Fri Jul 09 22:07:39 2010 +0400
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 6963811
+ * @summary Tests deadlock in PropertyEditorManager
+ * @author Sergey Malenkov
+ */
+
+import java.beans.PropertyEditorManager;
+import sun.beans.editors.StringEditor;
+
+public class Test6963811 implements Runnable {
+ private final long time;
+ private final boolean sync;
+
+ public Test6963811(long time, boolean sync) {
+ this.time = time;
+ this.sync = sync;
+ }
+
+ public void run() {
+ try {
+ Thread.sleep(this.time); // increase the chance of the deadlock
+ if (this.sync) {
+ synchronized (Test6963811.class) {
+ PropertyEditorManager.findEditor(Super.class);
+ }
+ }
+ else {
+ PropertyEditorManager.findEditor(Sub.class);
+ }
+ }
+ catch (Exception exception) {
+ exception.printStackTrace();
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ Thread[] threads = new Thread[2];
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(new Test6963811(0L, i > 0));
+ threads[i].start();
+ Thread.sleep(500L); // increase the chance of the deadlock
+ }
+ for (Thread thread : threads) {
+ thread.join();
+ }
+ }
+
+ public static class Super {
+ }
+
+ public static class Sub extends Super {
+ }
+
+ public static class SubEditor extends StringEditor {
+ public SubEditor() {
+ new Test6963811(1000L, true).run();
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/beans/XMLEncoder/Test6963811.java Fri Jul 09 22:07:39 2010 +0400
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 6963811
+ * @summary Tests deadlock in Encoder
+ * @author Sergey Malenkov
+ */
+
+import java.beans.Encoder;
+import java.beans.DefaultPersistenceDelegate;
+
+public class Test6963811 implements Runnable {
+ private static final Encoder ENCODER = new Encoder();
+ private final long time;
+ private final boolean sync;
+
+ public Test6963811(long time, boolean sync) {
+ this.time = time;
+ this.sync = sync;
+ }
+
+ public void run() {
+ try {
+ Thread.sleep(this.time); // increase the chance of the deadlock
+ if (this.sync) {
+ synchronized (Test6963811.class) {
+ ENCODER.getPersistenceDelegate(Super.class);
+ }
+ }
+ else {
+ ENCODER.getPersistenceDelegate(Sub.class);
+ }
+ }
+ catch (Exception exception) {
+ exception.printStackTrace();
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ Thread[] threads = new Thread[2];
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(new Test6963811(0L, i > 0));
+ threads[i].start();
+ Thread.sleep(500L); // increase the chance of the deadlock
+ }
+ for (Thread thread : threads) {
+ thread.join();
+ }
+ }
+
+ public static class Super {
+ }
+
+ public static class Sub extends Super {
+ }
+
+ public static class SubPersistenceDelegate extends DefaultPersistenceDelegate {
+ public SubPersistenceDelegate() {
+ new Test6963811(1000L, true).run();
+ }
+ }
+}