[uima-uimaj] branch bugfix/UIMA-6274-Setting-feature-on-builtin-JCas-cover-class-that-was-loaded-before-initializing-TypeSystemImpl-fails created (now aa003d9)

[re...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rec pushed a change to branch bugfix/UIMA-6274-Setting-feature-on-builtin-JCas-cover-class-that-was-loaded-before-initializing-TypeSystemImpl-fails
in repository https://gitbox.apache.org/repos/asf/uima-uimaj.git.


      at aa003d9  [UIMA-6274] Setting feature on builtin JCas cover class that was loaded before initializing TypeSystemImpl fails

This branch includes the following new commits:

     new aa003d9  [UIMA-6274] Setting feature on builtin JCas cover class that was loaded before initializing TypeSystemImpl fails

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[uima-uimaj] 01/01: [UIMA-6274] Setting feature on builtin JCas cover class that was loaded before initializing TypeSystemImpl fails

[re...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rec pushed a commit to branch bugfix/UIMA-6274-Setting-feature-on-builtin-JCas-cover-class-that-was-loaded-before-initializing-TypeSystemImpl-fails
in repository https://gitbox.apache.org/repos/asf/uima-uimaj.git

commit aa003d98cb8b993d68419adffedc7d687fda32c5
Author: Richard Eckart de Castilho <re...@apache.org>
AuthorDate: Thu Oct 15 23:20:45 2020 +0200

    [UIMA-6274] Setting feature on builtin JCas cover class that was loaded before initializing TypeSystemImpl fails
    
    - Introduce TypeSystemImpl.createCallSiteForBuiltIn() to provide special handling for the static TSI initialization being triggered by loading a built-in JCas cover class.
    - Updated the built-in JCas cover classes to use the new method to initialize their feature callsites
    - Added unit test
    - Fixed typo in comment
---
 .../org/apache/uima/cas/impl/FSClassRegistry.java  |  4 +-
 .../org/apache/uima/cas/impl/TypeSystemImpl.java   | 43 +++++++++++++++++++
 .../org/apache/uima/jcas/cas/AnnotationBase.java   |  2 +-
 .../java/org/apache/uima/jcas/cas/FSArrayList.java |  2 +-
 .../java/org/apache/uima/jcas/cas/FSHashSet.java   |  2 +-
 .../main/java/org/apache/uima/jcas/cas/Int2FS.java |  4 +-
 .../org/apache/uima/jcas/cas/IntegerArrayList.java |  2 +-
 .../org/apache/uima/jcas/cas/NonEmptyFSList.java   |  4 +-
 .../apache/uima/jcas/cas/NonEmptyFloatList.java    |  4 +-
 .../apache/uima/jcas/cas/NonEmptyIntegerList.java  |  4 +-
 .../apache/uima/jcas/cas/NonEmptyStringList.java   |  4 +-
 .../main/java/org/apache/uima/jcas/cas/Sofa.java   | 12 +++---
 .../uima/jcas/impl/JCasCoverClassLoadingTest.java  | 48 ++++++++++++++++++++++
 13 files changed, 113 insertions(+), 22 deletions(-)

diff --git a/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java b/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java
index d4cc147..38dc904 100644
--- a/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java
+++ b/uimaj-core/src/main/java/org/apache/uima/cas/impl/FSClassRegistry.java
@@ -294,7 +294,7 @@ public abstract class FSClassRegistry { // abstract to prevent instantiating; th
 
     reportErrors();
   }
-    
+
   private static void loadBuiltins(TypeImpl ti, ClassLoader cl, Map<String, JCasClassInfo> type2jcci, ArrayList<MutableCallSite> callSites_toSync) {
     String typeName = ti.getName();
     
@@ -1247,7 +1247,7 @@ public abstract class FSClassRegistry { // abstract to prevent instantiating; th
                           fi.getName(),
                            staticOffsetInClass,
                            fi.getAdjustedOffset()),
-                       staticOffsetInClass != -1);  // throw unless static offset is -1, in that case, a runtime error will occur if it is usedd
+                       staticOffsetInClass != -1);  // throw unless static offset is -1, in that case, a runtime error will occur if it is used
           }  // end of offset changed
         }  // end of feature check
       } // end of for loop
diff --git a/uimaj-core/src/main/java/org/apache/uima/cas/impl/TypeSystemImpl.java b/uimaj-core/src/main/java/org/apache/uima/cas/impl/TypeSystemImpl.java
index 5bcc51f..083bd05 100644
--- a/uimaj-core/src/main/java/org/apache/uima/cas/impl/TypeSystemImpl.java
+++ b/uimaj-core/src/main/java/org/apache/uima/cas/impl/TypeSystemImpl.java
@@ -2957,6 +2957,49 @@ public class TypeSystemImpl implements TypeSystem, TypeSystemMgr, LowLevelTypeSy
 //    callSitesForType.add(new AbstractMap.SimpleEntry<String, MutableCallSite>(featName, callSite));
     return callSite;
   }
+  
+  /**
+   * Creates and returns a new MutableCallSite for a built-in type. This handles the special case
+   * where the {@link #staticTsi} is initialized as a result of a builtin type JCas cover class
+   * being loaded in which case the callsite in the cover class is set after the {@link #staticTsi}
+   * already has been committed and therefore won't automatically get its callsites updated during
+   * the commit. So we need to create a proper callsite already pointing to the correct feature
+   * offset here.
+   * 
+   * @param clazz the JCas class
+   * @param featName the short name of the feature
+   * @return the created callsite
+   */
+  public final static MutableCallSite createCallSiteForBuiltIn(Class<? extends TOP> clazz, String featName) {
+    // If the static TSI has not yet been initialized, we assume that the initialization of the 
+    // static TSI was not triggered by the given JCas cover class. So we return a default callsite
+    // and trust that it will be properly updated when the static TSI is committed.
+    if (staticTsi == null) {
+      return createCallSite(clazz, featName);
+    }
+
+    // If the given JCas cover class not registered yet, we also assume that the initialization of 
+    // the static TSI was not triggered by the given JCas cover class.
+    TypeImpl type;
+    try {
+      int typeId = clazz.getField("typeIndexID").getInt(null);
+      type = (typeId >= staticTsi.jcasRegisteredTypes.size()) ? null
+              : staticTsi.jcasRegisteredTypes.get(typeId);
+      if (type == null) {
+        return createCallSite(clazz, featName);
+      }
+    }
+    catch (Exception e) {
+      throw new UIMARuntimeException(e, UIMARuntimeException.INTERNAL_ERROR, e);
+    }
+
+    // If the JCas class has already been registered and the static TSI is already there, we look
+    // up the proper feature offset in the static TSI
+    MutableCallSite callSite = new MutableCallSite(MethodType.methodType(int.class));
+    int adjustedOffset = type.getAdjOffset(featName);
+    callSite.setTarget(FSClassRegistry.getConstantIntMethodHandle(adjustedOffset));
+    return callSite;
+  }
 
   @Override
   public Iterator<Type> iterator() {
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/AnnotationBase.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/AnnotationBase.java
index 70feb82..00259aa 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/AnnotationBase.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/AnnotationBase.java
@@ -73,7 +73,7 @@ public class AnnotationBase extends TOP implements AnnotationBaseImpl {
   
   /* local data */
 //  public final static int _FI_sofa = TypeSystemImpl.getAdjustedFeatureOffset("sofa");
-  private final static CallSite _FC_sofa = TypeSystemImpl.createCallSite(AnnotationBase.class, "sofa");
+  private final static CallSite _FC_sofa = TypeSystemImpl.createCallSiteForBuiltIn(AnnotationBase.class, "sofa");
   private final static MethodHandle _FH_sofa = _FC_sofa.dynamicInvoker();
   
 //  private final Sofa _F_sofa;
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSArrayList.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSArrayList.java
index 73eb27e..91f2fb9 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSArrayList.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSArrayList.java
@@ -128,7 +128,7 @@ public class FSArrayList <T extends TOP> extends TOP implements
 
   /* Feature Adjusted Offsets */
 //  public final static int _FI_fsArray = TypeSystemImpl.getAdjustedFeatureOffset("fsArray");
-  private final static CallSite _FC_fsArray = TypeSystemImpl.createCallSite(FSArrayList.class, "fsArray");
+  private final static CallSite _FC_fsArray = TypeSystemImpl.createCallSiteForBuiltIn(FSArrayList.class, "fsArray");
   private final static MethodHandle _FH_fsArray = _FC_fsArray.dynamicInvoker();
 
    
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSHashSet.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSHashSet.java
index 360aefd..7ef1014 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSHashSet.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/FSHashSet.java
@@ -94,7 +94,7 @@ public class FSHashSet <T extends TOP> extends TOP implements
 
   /* Feature Adjusted Offsets */
 //  public final static int _FI_fsArray = TypeSystemImpl.getAdjustedFeatureOffset("fsArray");
-  private final static CallSite _FC_fsArray = TypeSystemImpl.createCallSite(FSHashSet.class, "fsArray");
+  private final static CallSite _FC_fsArray = TypeSystemImpl.createCallSiteForBuiltIn(FSHashSet.class, "fsArray");
   private final static MethodHandle _FH_fsArray = _FC_fsArray.dynamicInvoker();
 
    
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Int2FS.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Int2FS.java
index 0275544..2ac94fb 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Int2FS.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Int2FS.java
@@ -87,14 +87,14 @@ public class Int2FS <T extends TOP> extends TOP implements
 
 
   /* Feature Adjusted Offsets */
-  private final static CallSite _FC_fsArray = TypeSystemImpl.createCallSite(Int2FS.class, "fsArray");
+  private final static CallSite _FC_fsArray = TypeSystemImpl.createCallSiteForBuiltIn(Int2FS.class, "fsArray");
   private final static MethodHandle _FH_fsArray = _FC_fsArray.dynamicInvoker();
 
   public final static String _FeatName_intArray = "intArray";
 
 
   /* Feature Adjusted Offsets */
-  private final static CallSite _FC_intArray = TypeSystemImpl.createCallSite(Int2FS.class, "intArray");
+  private final static CallSite _FC_intArray = TypeSystemImpl.createCallSiteForBuiltIn(Int2FS.class, "intArray");
   private final static MethodHandle _FH_intArray = _FC_intArray.dynamicInvoker();
    
   /** Never called.  Disable default constructor
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/IntegerArrayList.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/IntegerArrayList.java
index c810a31..3375558 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/IntegerArrayList.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/IntegerArrayList.java
@@ -119,7 +119,7 @@ public class IntegerArrayList extends TOP implements
 
   /* Feature Adjusted Offsets */
 //  public final static int _FI_intArray = TypeSystemImpl.getAdjustedFeatureOffset("intArray");
-  private final static CallSite _FC_intArray = TypeSystemImpl.createCallSite(IntegerArrayList.class, "intArray");
+  private final static CallSite _FC_intArray = TypeSystemImpl.createCallSiteForBuiltIn(IntegerArrayList.class, "intArray");
   private final static MethodHandle _FH_intArray = _FC_intArray.dynamicInvoker();
 
    
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFSList.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFSList.java
index 39e0591..8668547 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFSList.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFSList.java
@@ -50,9 +50,9 @@ public class NonEmptyFSList<T extends TOP> extends FSList<T> implements NonEmpty
   
 //  public static final int _FI_head = TypeSystemImpl.getAdjustedFeatureOffset("head");
 //  public static final int _FI_tail = TypeSystemImpl.getAdjustedFeatureOffset("tail");
-  private final static CallSite _FC_head = TypeSystemImpl.createCallSite(NonEmptyFSList.class, "head");
+  private final static CallSite _FC_head = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyFSList.class, "head");
   private final static MethodHandle _FH_head = _FC_head.dynamicInvoker();
-  private final static CallSite _FC_tail = TypeSystemImpl.createCallSite(NonEmptyFSList.class, "tail");
+  private final static CallSite _FC_tail = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyFSList.class, "tail");
   private final static MethodHandle _FH_tail = _FC_tail.dynamicInvoker();
   
   
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFloatList.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFloatList.java
index be0f009..de74c6e 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFloatList.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyFloatList.java
@@ -50,9 +50,9 @@ public class NonEmptyFloatList extends FloatList implements NonEmptyList {
   
 //  public static final int _FI_head = TypeSystemImpl.getAdjustedFeatureOffset("head");
 //  public static final int _FI_tail = TypeSystemImpl.getAdjustedFeatureOffset("tail");
-  private final static CallSite _FC_head = TypeSystemImpl.createCallSite(NonEmptyFloatList.class, "head");
+  private final static CallSite _FC_head = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyFloatList.class, "head");
   private final static MethodHandle _FH_head = _FC_head.dynamicInvoker();
-  private final static CallSite _FC_tail = TypeSystemImpl.createCallSite(NonEmptyFloatList.class, "tail");
+  private final static CallSite _FC_tail = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyFloatList.class, "tail");
   private final static MethodHandle _FH_tail = _FC_tail.dynamicInvoker();
   
   /* local data */
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyIntegerList.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyIntegerList.java
index 8f2ff0e..eedf9f3 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyIntegerList.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyIntegerList.java
@@ -51,9 +51,9 @@ public class NonEmptyIntegerList extends IntegerList implements NonEmptyList {
 
 //  public final static int _FI_head = TypeSystemImpl.getAdjustedFeatureOffset("head");
 //  public final static int _FI_tail = TypeSystemImpl.getAdjustedFeatureOffset("tail");
-  private final static CallSite _FC_head = TypeSystemImpl.createCallSite(NonEmptyIntegerList.class, "head");
+  private final static CallSite _FC_head = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyIntegerList.class, "head");
   private final static MethodHandle _FH_head = _FC_head.dynamicInvoker();
-  private final static CallSite _FC_tail = TypeSystemImpl.createCallSite(NonEmptyIntegerList.class, "tail");
+  private final static CallSite _FC_tail = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyIntegerList.class, "tail");
   private final static MethodHandle _FH_tail = _FC_tail.dynamicInvoker();
 
 
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyStringList.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyStringList.java
index ef6abbd..eb69c65 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyStringList.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/NonEmptyStringList.java
@@ -50,9 +50,9 @@ public class NonEmptyStringList extends StringList implements Iterable<String>,
 
 //  public static final int _FI_head = TypeSystemImpl.getAdjustedFeatureOffset("head");
 //  public static final int _FI_tail = TypeSystemImpl.getAdjustedFeatureOffset("tail");
-  private final static CallSite _FC_head = TypeSystemImpl.createCallSite(NonEmptyStringList.class, "head");
+  private final static CallSite _FC_head = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyStringList.class, "head");
   private final static MethodHandle _FH_head = _FC_head.dynamicInvoker();
-  private final static CallSite _FC_tail = TypeSystemImpl.createCallSite(NonEmptyStringList.class, "tail");
+  private final static CallSite _FC_tail = TypeSystemImpl.createCallSiteForBuiltIn(NonEmptyStringList.class, "tail");
   private final static MethodHandle _FH_tail = _FC_tail.dynamicInvoker();
 
   
diff --git a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Sofa.java b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Sofa.java
index cf19c65..75b6947 100644
--- a/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Sofa.java
+++ b/uimaj-core/src/main/java/org/apache/uima/jcas/cas/Sofa.java
@@ -61,12 +61,12 @@ public class Sofa extends TOP implements SofaFSImpl {
 //  public final static int _FI_sofaString = TypeSystemImpl.getAdjustedFeatureOffset("sofaString");
 //  public final static int _FI_sofaURI    = TypeSystemImpl.getAdjustedFeatureOffset("sofaURI");
 
-  private final static CallSite _FC_sofaNum = TypeSystemImpl.createCallSite(Sofa.class, "sofaNum");
-  private final static CallSite _FC_sofaID = TypeSystemImpl.createCallSite(Sofa.class, "sofaID");
-  private final static CallSite _FC_mimeType = TypeSystemImpl.createCallSite(Sofa.class, "mimeType");
-  private final static CallSite _FC_sofaArray = TypeSystemImpl.createCallSite(Sofa.class, "sofaArray");
-  private final static CallSite _FC_sofaString = TypeSystemImpl.createCallSite(Sofa.class, "sofaString");
-  private final static CallSite _FC_sofaURI = TypeSystemImpl.createCallSite(Sofa.class, "sofaURI");
+  private final static CallSite _FC_sofaNum = TypeSystemImpl.createCallSiteForBuiltIn(Sofa.class, "sofaNum");
+  private final static CallSite _FC_sofaID = TypeSystemImpl.createCallSiteForBuiltIn(Sofa.class, "sofaID");
+  private final static CallSite _FC_mimeType = TypeSystemImpl.createCallSiteForBuiltIn(Sofa.class, "mimeType");
+  private final static CallSite _FC_sofaArray = TypeSystemImpl.createCallSiteForBuiltIn(Sofa.class, "sofaArray");
+  private final static CallSite _FC_sofaString = TypeSystemImpl.createCallSiteForBuiltIn(Sofa.class, "sofaString");
+  private final static CallSite _FC_sofaURI = TypeSystemImpl.createCallSiteForBuiltIn(Sofa.class, "sofaURI");
 
   private final static MethodHandle _FH_sofaNum = _FC_sofaNum.dynamicInvoker();
   private final static MethodHandle _FH_sofaID = _FC_sofaID.dynamicInvoker();
diff --git a/uimaj-core/src/test/java/org/apache/uima/jcas/impl/JCasCoverClassLoadingTest.java b/uimaj-core/src/test/java/org/apache/uima/jcas/impl/JCasCoverClassLoadingTest.java
new file mode 100644
index 0000000..b4df0d2
--- /dev/null
+++ b/uimaj-core/src/test/java/org/apache/uima/jcas/impl/JCasCoverClassLoadingTest.java
@@ -0,0 +1,48 @@
+/*
+ * 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.uima.jcas.impl;
+
+import org.apache.uima.cas.impl.TypeSystemImpl;
+import org.apache.uima.jcas.JCas;
+import org.apache.uima.jcas.cas.Sofa;
+import org.apache.uima.resource.metadata.TypeSystemDescription;
+import org.apache.uima.resource.metadata.impl.TypeSystemDescription_impl;
+import org.apache.uima.util.CasCreationUtils;
+import org.junit.Test;
+
+public class JCasCoverClassLoadingTest {
+  
+  /**
+   * This test failed when we were using {@link TypeSystemImpl#createCallSite(Class, String)}
+   * instead of {@link TypeSystemImpl#createCallSiteForBuiltIn(Class, String)} to initialize 
+   * built-in JCas cover classes.
+   * 
+   * @see <a href="https://issues.apache.org/jira/browse/UIMA-6274">UIMA-6274</a>
+   */
+  @Test
+  public void thatLoadingABuiltInCoverClassBeforeTypeSystemImplDoesNotBreakTheClass() throws Exception
+  {
+    Class.forName(Sofa.class.getName());
+
+    TypeSystemDescription tsd = new TypeSystemDescription_impl();
+    JCas jcas = CasCreationUtils.createCas(tsd, null, null).getJCas();
+    // Originally an exception was thrown in the following line
+    jcas.setDocumentText("test text");
+  }
+}