From c2b942f0242d80e876a9dd712b539a865825c520 Mon Sep 17 00:00:00 2001 From: Ilya Kasnacheev Date: Fri, 25 May 2018 17:25:04 +0300 Subject: [PATCH] IGNITE-8547 Use JVM serialization for enum values with OptimizedMarshaller, avoid deadlock. --- .../ignite/internal/binary/BinaryUtils.java | 19 +- .../builder/BinaryBuilderSerializer.java | 3 +- .../OptimizedObjectOutputStream.java | 4 +- .../ignite/internal/util/IgniteUtils.java | 15 ++ .../MarshallerEnumDeadlockMultiJvmTest.java | 190 ++++++++++++++++++ .../IgniteMarshallerSelfTestSuite.java | 2 + 6 files changed, 214 insertions(+), 19 deletions(-) create mode 100644 modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java index 1f167f5f08080..082cc206e3231 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java @@ -600,7 +600,7 @@ public static byte typeByClass(Class cls) { if (type != null) return type; - if (isEnum(cls)) + if (U.isEnum(cls)) return GridBinaryMarshaller.ENUM; if (cls.isArray()) @@ -1141,7 +1141,7 @@ else if (isSpecialCollection(cls)) return BinaryWriteMode.COL; else if (isSpecialMap(cls)) return BinaryWriteMode.MAP; - else if (isEnum(cls)) + else if (U.isEnum(cls)) return BinaryWriteMode.ENUM; else if (cls == BinaryEnumObjectImpl.class) return BinaryWriteMode.BINARY_ENUM; @@ -1174,21 +1174,6 @@ public static boolean isSpecialMap(Class cls) { return HashMap.class.equals(cls) || LinkedHashMap.class.equals(cls); } - /** - * Check if class represents a Enum. - * - * @param cls Class. - * @return {@code True} if this is a Enum class. - */ - public static boolean isEnum(Class cls) { - if (cls.isEnum()) - return true; - - Class sCls = cls.getSuperclass(); - - return sCls != null && sCls.isEnum(); - } - /** * @return Value. */ diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java index 018444c65123a..42f6873bf1548 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java @@ -27,6 +27,7 @@ import org.apache.ignite.internal.binary.BinaryUtils; import org.apache.ignite.internal.binary.BinaryWriterExImpl; import org.apache.ignite.internal.binary.GridBinaryMarshaller; +import org.apache.ignite.internal.util.IgniteUtils; /** * @@ -110,7 +111,7 @@ public void writeValue(BinaryWriterExImpl writer, Object val, boolean forceCol, return; } - if (BinaryUtils.isEnum(val.getClass())) { + if (IgniteUtils.isEnum(val.getClass())) { String clsName = ((Enum)val).getDeclaringClass().getName(); int typeId = writer.context().typeId(clsName); diff --git a/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java b/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java index 66da2da7eb908..fadbec61fad02 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/marshaller/optimized/OptimizedObjectOutputStream.java @@ -42,6 +42,7 @@ import org.apache.ignite.internal.util.GridHandleTable; import org.apache.ignite.internal.util.io.GridDataOutput; import org.apache.ignite.internal.util.typedef.F; +import org.apache.ignite.internal.util.typedef.internal.U; import org.apache.ignite.lang.IgniteBiTuple; import org.apache.ignite.marshaller.MarshallerContext; @@ -180,7 +181,8 @@ private void writeObject0(Object obj) throws IOException { if (obj == null) writeByte(NULL); else { - if (obj instanceof Throwable && !(obj instanceof Externalizable)) { + if (obj instanceof Throwable && !(obj instanceof Externalizable) || U.isEnum(obj.getClass())) { + // Avoid problems with differing Enum objects or Enum implementation class deadlocks. writeByte(JDK); try { diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java index 2defefad4721e..1e34c2d66a93e 100755 --- a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java @@ -6156,6 +6156,21 @@ public static boolean isJdk(Class cls) { return s.startsWith("java.") || s.startsWith("javax."); } + /** + * Check if given class represents a Enum. + * + * @param cls Class to check. + * @return {@code True} if this is a Enum class. + */ + public static boolean isEnum(Class cls) { + if (cls.isEnum()) + return true; + + Class sCls = cls.getSuperclass(); + + return sCls != null && sCls.isEnum(); + } + /** * Converts {@link InterruptedException} to {@link IgniteCheckedException}. * diff --git a/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java b/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java new file mode 100644 index 0000000000000..2b68ab03b0dd5 --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/marshaller/MarshallerEnumDeadlockMultiJvmTest.java @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.marshaller; + +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import javax.cache.configuration.Factory; +import org.apache.ignite.Ignite; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.internal.binary.BinaryMarshaller; +import org.apache.ignite.internal.marshaller.optimized.OptimizedMarshaller; +import org.apache.ignite.lang.IgniteCallable; +import org.apache.ignite.marshaller.jdk.JdkMarshaller; +import org.apache.ignite.resources.IgniteInstanceResource; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; + +/** + * Contains test of Enum marshalling with various {@link Marshaller}s. See IGNITE-8547 for details. + */ +public class MarshallerEnumDeadlockMultiJvmTest extends GridCommonAbstractTest { + /** */ + private Factory marshFactory; + + /** {@inheritDoc} */ + @Override protected IgniteConfiguration getConfiguration(String instanceName) throws Exception { + return super.getConfiguration(instanceName).setMarshaller(marshFactory.create()); + } + + /** */ + public void testJdkMarshaller() throws Exception { + marshFactory = new JdkMarshallerFactory(); + + runRemoteUnmarshal(); + } + + /** */ + public void testOptimizedMarshaller() throws Exception { + marshFactory = new OptimizedMarshallerFactory(); + + runRemoteUnmarshal(); + } + + /** */ + public void testBinaryMarshaller() throws Exception { + marshFactory = new BinaryMarshallerFactory(); + + runRemoteUnmarshal(); + } + + /** */ + private void runRemoteUnmarshal() throws Exception { + Ignite ignite = startGrid(0); + + byte[] one = ignite.configuration().getMarshaller().marshal(DeclaredBodyEnum.ONE); + byte[] two = ignite.configuration().getMarshaller().marshal(DeclaredBodyEnum.TWO); + + startGrid(1); + + ignite.compute(ignite.cluster().forRemotes()).call(new UnmarshalCallable(one, two)); + } + + /** {@inheritDoc} */ + @Override protected boolean isMultiJvm() { + return true; + } + + @Override protected void afterTest() throws Exception { + stopAllGrids(); + } + + /** */ + private static class OptimizedMarshallerFactory implements Factory { + /** {@inheritDoc} */ + @Override public Marshaller create() { + return new OptimizedMarshaller(false); + } + } + + /** */ + private static class BinaryMarshallerFactory implements Factory { + /** {@inheritDoc} */ + @Override public Marshaller create() { + return new BinaryMarshaller(); + } + } + + /** */ + private static class JdkMarshallerFactory implements Factory { + /** {@inheritDoc} */ + @Override public Marshaller create() { + return new JdkMarshaller(); + } + } + + /** + * Attempts to unmarshal both in-built and inner-class enum values at exactly the same time in multiple threads. + */ + private static class UnmarshalCallable implements IgniteCallable { + /** */ + private final byte[] one; + /** */ + private final byte[] two; + /** */ + @IgniteInstanceResource + private Ignite ign; + + /** */ + public UnmarshalCallable(byte[] one, byte[] two) { + this.one = one; + this.two = two; + } + + /** {@inheritDoc} */ + @Override public Object call() throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(2); + + final CyclicBarrier start = new CyclicBarrier(2); + + for (int i = 0; i < 2; i++) { + final int ii = i; + + executor.execute(new Runnable() { + @Override public void run() { + try { + start.await(); + + if (ii == 0) + ign.configuration().getMarshaller().unmarshal(one, null); + else + ign.configuration().getMarshaller().unmarshal(two, null); + } + catch (Exception e) { + e.printStackTrace(); + } + } + }); + } + + try { + executor.shutdown(); + + executor.awaitTermination(5, TimeUnit.SECONDS); + + if (!executor.isTerminated()) + throw new IllegalStateException("Failed to wait for completion"); + } + catch (Exception te) { + throw new IllegalStateException("Failed to wait for completion", te); + } + + return null; + } + } + + /** */ + public enum DeclaredBodyEnum { + ONE, + TWO { + /** {@inheritDoc} */ + @Override public boolean isSupported() { + return false; + } + }; + + /** + * A bogus method. + */ + public boolean isSupported() { + return true; + } + } +} diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java index 2d7f64b50b74b..0eb50d739ff9e 100644 --- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java +++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteMarshallerSelfTestSuite.java @@ -31,6 +31,7 @@ import org.apache.ignite.internal.util.io.GridUnsafeDataInputOutputByteOrderSelfTest; import org.apache.ignite.internal.util.io.GridUnsafeDataOutputArraySizingSelfTest; import org.apache.ignite.marshaller.GridMarshallerMappingConsistencyTest; +import org.apache.ignite.marshaller.MarshallerEnumDeadlockMultiJvmTest; import org.apache.ignite.marshaller.jdk.GridJdkMarshallerSelfTest; import org.apache.ignite.testframework.GridTestUtils; @@ -67,6 +68,7 @@ public static TestSuite suite(Set ignoredTests) throws Exception { GridTestUtils.addTestIfNeeded(suite, GridHandleTableSelfTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, OptimizedMarshallerPooledSelfTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, GridMarshallerMappingConsistencyTest.class, ignoredTests); + GridTestUtils.addTestIfNeeded(suite, MarshallerEnumDeadlockMultiJvmTest.class, ignoredTests); return suite; }