IGNITE-8926 Fixed deadlock on metadata registration and concurrent CQ…

… modification - Fixes apache#4507.
gridgain · Aug 17, 2018 · 62a50ba · 62a50ba
1 parent 02a874e
commit 62a50ba
Show file tree

Hide file tree

Showing 33 changed files with 711 additions and 98 deletions.
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/UnregisteredBinaryTypeException.java b/modules/core/src/main/java/org/apache/ignite/internal/UnregisteredBinaryTypeException.java
@@ -0,0 +1,96 @@
+/*
+ * 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.internal;
+
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Exception thrown during serialization if binary metadata isn't registered and it's registration isn't allowed.
+ */
+public class UnregisteredBinaryTypeException extends IgniteException {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** */
+    private final int typeId;
+
+    /** */
+    private final BinaryMetadata binaryMetadata;
+
+    /**
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(int typeId, BinaryMetadata binaryMetadata) {
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @param msg Error message.
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(String msg, int typeId,
+        BinaryMetadata binaryMetadata) {
+        super(msg);
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @param cause Non-null throwable cause.
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(Throwable cause, int typeId,
+        BinaryMetadata binaryMetadata) {
+        super(cause);
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @param msg Error message.
+     * @param cause Non-null throwable cause.
+     * @param typeId Type ID.
+     * @param binaryMetadata Binary metadata.
+     */
+    public UnregisteredBinaryTypeException(String msg, @Nullable Throwable cause, int typeId,
+        BinaryMetadata binaryMetadata) {
+        super(msg, cause);
+        this.typeId = typeId;
+        this.binaryMetadata = binaryMetadata;
+    }
+
+    /**
+     * @return Type ID.
+     */
+    public int typeId() {
+        return typeId;
+    }
+
+    /**
+     * @return Binary metadata.
+     */
+    public BinaryMetadata binaryMetadata() {
+        return binaryMetadata;
+    }
+}
diff --git a/...es/core/src/main/java/org/apache/ignite/internal/binary/BinaryCachingMetadataHandler.java b/...es/core/src/main/java/org/apache/ignite/internal/binary/BinaryCachingMetadataHandler.java
@@ -46,7 +46,7 @@ private BinaryCachingMetadataHandler() {
     }
 
     /** {@inheritDoc} */
-    @Override public synchronized void addMeta(int typeId, BinaryType type) throws BinaryObjectException {
+    @Override public synchronized void addMeta(int typeId, BinaryType type, boolean failIfUnregistered) throws BinaryObjectException {
         synchronized (this) {
             BinaryType oldType = metas.put(typeId, type);
 

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java
@@ -40,6 +40,8 @@
 import org.apache.ignite.binary.BinaryReflectiveSerializer;
 import org.apache.ignite.binary.BinarySerializer;
 import org.apache.ignite.binary.Binarylizable;
+import org.apache.ignite.internal.UnregisteredClassException;
+import org.apache.ignite.internal.UnregisteredBinaryTypeException;
 import org.apache.ignite.internal.marshaller.optimized.OptimizedMarshaller;
 import org.apache.ignite.internal.processors.cache.CacheObjectImpl;
 import org.apache.ignite.internal.processors.query.QueryUtils;
@@ -773,7 +775,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
                                     BinaryMetadata meta = new BinaryMetadata(typeId, typeName, collector.meta(),
                                         affKeyFieldName, Collections.singleton(newSchema), false, null);
 
-                                    ctx.updateMetadata(typeId, meta);
+                                    ctx.updateMetadata(typeId, meta, writer.failIfUnregistered());
 
                                     schemaReg.addSchema(newSchema.schemaId(), newSchema);
                                 }
@@ -794,7 +796,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
                         BinaryMetadata meta = new BinaryMetadata(typeId, typeName, stableFieldsMeta,
                             affKeyFieldName, Collections.singleton(stableSchema), false, null);
 
-                        ctx.updateMetadata(typeId, meta);
+                        ctx.updateMetadata(typeId, meta, writer.failIfUnregistered());
 
                         schemaReg.addSchema(stableSchema.schemaId(), stableSchema);
 
@@ -823,6 +825,9 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
             }
         }
         catch (Exception e) {
+            if (e instanceof UnregisteredBinaryTypeException || e instanceof UnregisteredClassException)
+                throw e;
+
             String msg;
 
             if (S.INCLUDE_SENSITIVE && !F.isEmpty(typeName))

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java
@@ -652,7 +652,7 @@ else if (!desc.registered()) {
                         schemas, desc0.isEnum(),
                         cls.isEnum() ? enumMap(cls) : null);
 
-                    metaHnd.addMeta(desc0.typeId(), meta.wrap(this));
+                    metaHnd.addMeta(desc0.typeId(), meta.wrap(this), false);
 
                     return desc0;
                 }
@@ -800,7 +800,7 @@ private BinaryClassDescriptor registerUserClassDescriptor(Class<?> cls, boolean
 
         if (!deserialize)
             metaHnd.addMeta(typeId, new BinaryMetadata(typeId, typeName, desc.fieldsMeta(), affFieldName, null,
-                desc.isEnum(), cls.isEnum() ? enumMap(cls) : null).wrap(this));
+                desc.isEnum(), cls.isEnum() ? enumMap(cls) : null).wrap(this), false);
 
         descByCls.put(cls, desc);
 
@@ -1169,7 +1169,7 @@ public void registerUserType(String clsName,
         }
 
         metaHnd.addMeta(id,
-            new BinaryMetadata(id, typeName, fieldsMeta, affKeyFieldName, null, isEnum, enumMap).wrap(this));
+            new BinaryMetadata(id, typeName, fieldsMeta, affKeyFieldName, null, isEnum, enumMap).wrap(this), false);
     }
 
     /**
@@ -1324,10 +1324,11 @@ public BinaryIdentityResolver identity(int typeId) {
     /**
      * @param typeId Type ID.
      * @param meta Meta data.
+     * @param failIfUnregistered Fail if unregistered.
      * @throws BinaryObjectException In case of error.
      */
-    public void updateMetadata(int typeId, BinaryMetadata meta) throws BinaryObjectException {
-        metaHnd.addMeta(typeId, meta.wrap(this));
+    public void updateMetadata(int typeId, BinaryMetadata meta, boolean failIfUnregistered) throws BinaryObjectException {
+        metaHnd.addMeta(typeId, meta.wrap(this), failIfUnregistered);
     }
 
     /**

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java
@@ -26,6 +26,7 @@
 import java.util.Map;
 import java.util.UUID;
 import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.internal.UnregisteredClassException;
 import org.apache.ignite.internal.util.GridUnsafe;
 import org.apache.ignite.internal.util.typedef.F;
 import org.apache.ignite.internal.util.typedef.internal.S;
@@ -155,6 +156,9 @@ public void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectExce
             write0(obj, writer);
         }
         catch (Exception ex) {
+            if (ex instanceof UnregisteredClassException)
+                throw ex;
+
             if (S.INCLUDE_SENSITIVE && !F.isEmpty(name))
                 throw new BinaryObjectException("Failed to write field [name=" + name + ']', ex);
             else

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java
@@ -30,9 +30,10 @@ public interface BinaryMetadataHandler {
      *
      * @param typeId Type ID.
      * @param meta Metadata.
+     * @param failIfUnregistered Fail if unregistered.
      * @throws BinaryObjectException In case of error.
      */
-    public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException;
+    public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException;
 
     /**
      * Gets meta data for provided type ID.

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryNoopMetadataHandler.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryNoopMetadataHandler.java
@@ -43,7 +43,7 @@ private BinaryNoopMetadataHandler() {
     }
 
     /** {@inheritDoc} */
-    @Override public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException {
+    @Override public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException {
         // No-op.
     }
 

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java
@@ -115,6 +115,13 @@ public BinaryWriterExImpl(BinaryContext ctx, BinaryOutputStream out, BinaryWrite
         start = out.position();
     }
 
+    /**
+     * @return Fail if unregistered flag value.
+     */
+    public boolean failIfUnregistered() {
+        return failIfUnregistered;
+    }
+
     /**
      * @param failIfUnregistered Fail if unregistered.
      */
@@ -503,6 +510,8 @@ public void doWriteObject(@Nullable Object obj) throws BinaryObjectException {
         else {
             BinaryWriterExImpl writer = new BinaryWriterExImpl(ctx, out, schema, handles());
 
+            writer.failIfUnregistered(failIfUnregistered);
+
             writer.marshal(obj);
         }
     }
@@ -1492,6 +1501,8 @@ void writeBinaryObjectField(@Nullable BinaryObjectImpl po) throws BinaryObjectEx
         else {
             BinaryWriterExImpl writer = new BinaryWriterExImpl(ctx, out, schema, null);
 
+            writer.failIfUnregistered(failIfUnregistered);
+
             writer.marshal(obj);
         }
     }
@@ -1915,6 +1926,8 @@ boolean tryWriteAsHandle(Object obj) {
     public BinaryWriterExImpl newWriter(int typeId) {
         BinaryWriterExImpl res = new BinaryWriterExImpl(ctx, out, schema, handles());
 
+        res.failIfUnregistered(failIfUnregistered);
+
         res.typeId(typeId);
 
         return res;

diff --git a/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java b/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
@@ -126,7 +126,7 @@ public void writeValue(BinaryWriterExImpl writer, Object val, boolean forceCol,
 
             BinaryMetadata meta = new BinaryMetadata(typeId, typeName, null, null, null, true, enumMap);
 
-            writer.context().updateMetadata(typeId, meta);
+            writer.context().updateMetadata(typeId, meta, writer.failIfUnregistered());
 
             // Need register class for marshaller to be able to deserialize enum value.
             writer.context().descriptorForClass(((Enum)val).getDeclaringClass(), false, false);

diff --git a/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java b/...core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java
@@ -36,6 +36,7 @@
 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.thread.IgniteThread;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Collection;
@@ -174,6 +175,12 @@ public BinaryObjectBuilderImpl(BinaryObjectImpl obj) {
     /** {@inheritDoc} */
     @Override public BinaryObject build() {
         try (BinaryWriterExImpl writer = new BinaryWriterExImpl(ctx)) {
+            Thread curThread = Thread.currentThread();
+
+            if (curThread instanceof IgniteThread)
+                writer.failIfUnregistered(((IgniteThread)curThread).executingEntryProcessor() &&
+                    ((IgniteThread)curThread).holdsTopLock());
+
             writer.typeId(typeId);
 
             BinaryBuilderSerializer serializationCtx = new BinaryBuilderSerializer();
@@ -360,7 +367,7 @@ else if (readCache == null) {
                 ctx.registerUserClassName(typeId, typeName);
 
                 ctx.updateMetadata(typeId, new BinaryMetadata(typeId, typeName, fieldsMeta, affFieldName0,
-                    Collections.singleton(curSchema), false, null));
+                    Collections.singleton(curSchema), false, null), writer.failIfUnregistered());
 
                 schemaReg.addSchema(curSchema.schemaId(), curSchema);
             }

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientBinary.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientBinary.java
@@ -160,7 +160,7 @@ class ClientBinary implements IgniteBinary {
 
         int typeId = ctx.typeId(typeName);
 
-        ctx.updateMetadata(typeId, new BinaryMetadata(typeId, typeName, null, null, null, true, vals));
+        ctx.updateMetadata(typeId, new BinaryMetadata(typeId, typeName, null, null, null, true, vals), false);
 
         return ctx.metadata(typeId);
     }

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
@@ -233,7 +233,7 @@ private class ClientBinaryMetadataHandler implements BinaryMetadataHandler {
         private final BinaryMetadataHandler cache = BinaryCachingMetadataHandler.create();
 
         /** {@inheritDoc} */
-        @Override public void addMeta(int typeId, BinaryType meta) throws BinaryObjectException {
+        @Override public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException {
             if (cache.metadata(typeId) == null) {
                 try {
                     ch.request(
@@ -246,7 +246,7 @@ private class ClientBinaryMetadataHandler implements BinaryMetadataHandler {
                 }
             }
 
-            cache.addMeta(typeId, meta); // merge
+            cache.addMeta(typeId, meta, failIfUnregistered); // merge
         }
 
         /** {@inheritDoc} */
@@ -259,7 +259,7 @@ private class ClientBinaryMetadataHandler implements BinaryMetadataHandler {
                 if (meta0 != null) {
                     meta = new BinaryTypeImpl(marsh.context(), meta0);
 
-                    cache.addMeta(typeId, meta);
+                    cache.addMeta(typeId, meta, false);
                 }
             }
 

diff --git a/...re/src/main/java/org/apache/ignite/internal/processors/cache/CacheInvokeDirectResult.java b/...re/src/main/java/org/apache/ignite/internal/processors/cache/CacheInvokeDirectResult.java
@@ -40,6 +40,10 @@ public class CacheInvokeDirectResult implements Message {
     /** */
     private KeyCacheObject key;
 
+    /** */
+    @GridToStringInclude
+    private transient Object unprepareRes;
+
     /** */
     @GridToStringInclude
     private CacheObject res;
@@ -68,6 +72,22 @@ public CacheInvokeDirectResult(KeyCacheObject key, CacheObject res) {
         this.res = res;
     }
 
+    /**
+     * Constructs CacheInvokeDirectResult with unprepared res, to avoid object marshaling while holding topology locks.
+     *
+     * @param key Key.
+     * @param res Result.
+     * @return a new instance of CacheInvokeDirectResult.
+     */
+    static CacheInvokeDirectResult lazyResult(KeyCacheObject key, Object res) {
+        CacheInvokeDirectResult res0 = new CacheInvokeDirectResult();
+
+        res0.key = key;
+        res0.unprepareRes = res;
+
+        return res0;
+    }
+
     /**
      * @param key Key.
      * @param err Exception thrown by {@link EntryProcessor#process(MutableEntry, Object...)}.
@@ -120,6 +140,12 @@ public void prepareMarshal(GridCacheContext ctx) throws IgniteCheckedException {
             }
         }
 
+        if (unprepareRes != null) {
+            res = ctx.toCacheObject(unprepareRes);
+
+            unprepareRes = null;
+        }
+
         if (res != null)
             res.prepareMarshal(ctx.cacheObjectContext());
     }