Fix an intermittent deadlock when DocService is loaded for a Thrift service

thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftDescriptiveTypeInfoProvider.java

@@ -136,7 +136,6 @@ static ExceptionInfo newExceptionInfo(Class<? extends TException> exceptionClass
  return new ExceptionInfo(name, fields);
  }
 
- @VisibleForTesting
  static FieldInfo newFieldInfo(Class<?> parentType, FieldMetaData fieldMetaData) {
  requireNonNull(fieldMetaData, "fieldMetaData");
  final FieldValueMetaData fieldValueMetaData = fieldMetaData.valueMetaData;
@@ -318,8 +317,7 @@ private static boolean hasTypeDef(FieldValueMetaData valueMetadata) {
  static <T extends TBase<T, F>, F extends TFieldIdEnum> StructInfo newStructInfo(Class<?> structClass) {
  final String name = structClass.getName();
 
- //noinspection unchecked
- final Map<?, FieldMetaData> metaDataMap = FieldMetaData.getStructMetaDataMap((Class<T>) structClass);
+ final Map<?, FieldMetaData> metaDataMap = ThriftMetadataAccess.getStructMetaDataMap(structClass);
  final List<FieldInfo> fields =
  metaDataMap.values().stream()
  .map(fieldMetaData -> newFieldInfo(structClass, fieldMetaData))

thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePlugin.java

@@ -239,18 +239,16 @@ private static <T extends TBase<T, F>, F extends TFieldIdEnum> MethodInfo newMet
  requireNonNull(exceptionClasses, "exceptionClasses");
  requireNonNull(endpoints, "endpoints");
 
- //noinspection unchecked,RedundantCast
  final List<FieldInfo> parameters =
- FieldMetaData.getStructMetaDataMap((Class<T>) argsClass).values().stream()
- .map(fieldMetaData -> newFieldInfo(argsClass, fieldMetaData))
- .collect(toImmutableList());
+ ThriftMetadataAccess.getStructMetaDataMap(argsClass).values().stream()
+  .map(fieldMetaData -> newFieldInfo(argsClass, fieldMetaData))
+  .collect(toImmutableList());
 
  // Find the 'success' field.
  FieldInfo fieldInfo = null;
  if (resultClass != null) { // Function isn't "oneway" function
- //noinspection unchecked,RedundantCast
- final Map<? extends TFieldIdEnum, FieldMetaData> resultMetaData =
- FieldMetaData.getStructMetaDataMap((Class<T>) resultClass);
+ final Map<?, FieldMetaData> resultMetaData =
+ ThriftMetadataAccess.getStructMetaDataMap(resultClass);
 
  for (FieldMetaData fieldMetaData : resultMetaData.values()) {
  if ("success".equals(fieldMetaData.fieldName)) {

thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftMetadataAccess.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2023 LINE Corporation
+ *
+ * LINE Corporation 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:
+ *
+ * https://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 com.linecorp.armeria.internal.server.thrift;
+
+import java.io.BufferedReader;
+import java.io.InputStreamReader;
+import java.net.URL;
+import java.util.Enumeration;
+import java.util.Map;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.thrift.TBase;
+import org.apache.thrift.TFieldIdEnum;
+import org.apache.thrift.meta_data.FieldMetaData;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
+
+final class ThriftMetadataAccess {
+
+ private static final Logger logger = LoggerFactory.getLogger(ThriftMetadataAccess.class);
+
+ private static boolean preInitializeThriftClass;
+ private static final Pattern preInitializeTargetPattern =
+ Pattern.compile("^armeria-thrift0\\.(\\d+)\\..*$");
+
+ static {
+ try {
+ final Enumeration<URL> versionPropertiesUrls =
+ ThriftMetadataAccess.class.getClassLoader().getResources(
+ "META-INF/com.linecorp.armeria.versions.properties");
+ if (!versionPropertiesUrls.hasMoreElements()) {
+ // versions.properties was not found
+ logger.trace("Unable to determine the 'armeria-thrift' version. Please consider " +
+ "adding 'META-INF/com.linecorp.armeria.versions.properties' to the " +
+ "classpath to avoid unexpected issues.");
+ }
+ boolean preInitializeThriftClass = false;
+ while (versionPropertiesUrls.hasMoreElements()) {
+ final URL url = versionPropertiesUrls.nextElement();
+ try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream()))) {
+ final Properties props = new Properties();
+ props.load(reader);
+ preInitializeThriftClass = needsPreInitialization(props);
+ if (preInitializeThriftClass) {
+ break;
+ }
+ }
+ }
+ ThriftMetadataAccess.preInitializeThriftClass = preInitializeThriftClass;
+ } catch (Exception e) {
+ logger.debug("Unexpected exception while determining the 'armeria-thrift' version: ", e);
+ }
+ }
+
+ @VisibleForTesting
+ static boolean needsPreInitialization(Properties props) {
+ for (String key : props.stringPropertyNames()) {
+ final Matcher matcher = preInitializeTargetPattern.matcher(key);
+ if (!matcher.matches()) {
+ continue;
+ }
+ final int version = Integer.parseInt(matcher.group(1));
+ if (version <= 14) {
+ logger.trace("Pre-initializing thrift metadata due to 'armeria-thrift0.{}'", version);
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <T extends TBase<T, F>, F extends TFieldIdEnum>
+ Map<?, FieldMetaData> getStructMetaDataMap(Class<?> clazz) {
+ // Pre-initialize classes if there is a jar in the classpath with armeria-thrift <= 0.14
+ // See the following issue for the motivation of pre-initializing classes
+ // https://issues.apache.org/jira/browse/THRIFT-5430
+ if (preInitializeThriftClass) {
+ try {
+ Class.forName(clazz.getName(), true, clazz.getClassLoader());
+ } catch (ClassNotFoundException e) {
+ // Another exception will probably be raised in the actual getStructMetaDataMap so just
+ // logging for at this point.
+ logger.trace("Unexpected exception while initializing class {}: ", clazz, e);
+ }
+ }
+ return FieldMetaData.getStructMetaDataMap((Class<T>) clazz);
+ }
+
+ private ThriftMetadataAccess() {}
+}

thrift/thrift0.13/src/test/java/com/linecorp/armeria/internal/server/thrift/ThriftClassLoadingTest.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2023 LINE Corporation
+ *
+ * LINE Corporation 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:
+ *
+ * https://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 com.linecorp.armeria.internal.server.thrift;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.Test;
+
+import net.bytebuddy.dynamic.ClassFileLocator.ForClassLoader;
+
+import com.linecorp.armeria.service.test.thrift.main.FooStruct;
+
+class ThriftClassLoadingTest {
+
+ // see https://issues.apache.org/jira/browse/THRIFT-5430
+ @Test
+ void testDeadlock() throws Exception {
+ for (int i = 0; i < 20; i++) {
+ final ExecutorService e1 = Executors.newSingleThreadExecutor();
+ final ExecutorService e2 = Executors.newSingleThreadExecutor();
+ final ClassLoader classLoader = new SimpleClassLoader(FooStruct.class);
+ @SuppressWarnings("unchecked")
+ final Class<FooStruct> aClass =
+ (Class<FooStruct>) Class.forName(FooStruct.class.getName(), false, classLoader);
+ e1.submit(() -> ThriftDescriptiveTypeInfoProvider.newStructInfo(aClass));
+ e2.submit(() -> Class.forName(FooStruct.class.getName(), true, classLoader));
+ e1.shutdown();
+ e2.shutdown();
+ // unfortunately if a deadlock did occur the threads are in an uninterruptible state
+ // which means the threads cannot be cleaned up
+ assertThat(e1.awaitTermination(10, TimeUnit.SECONDS)).isTrue();
+ assertThat(e2.awaitTermination(10, TimeUnit.SECONDS)).isTrue();
+ }
+ }
+
+ /**
+ * A simple class loader used to re-initialize a class.
+ */
+ private static class SimpleClassLoader extends ClassLoader {
+
+ private final Class<?> targetClass;
+ private final Map<String, Class<?>> memo = new HashMap<>();
+
+ SimpleClassLoader(Class<?> targetClass) {
+ this.targetClass = targetClass;
+ }
+
+ @Override
+ public synchronized Class<?> loadClass(String name) throws ClassNotFoundException {
+ if (!name.startsWith(targetClass.getName())) {
+ return super.loadClass(name);
+ }
+ if (memo.containsKey(name)) {
+ return memo.get(name);
+ }
+ final byte[] bytes = ForClassLoader.read(Class.forName(name));
+ final Class<?> clazz = defineClass(name, bytes, 0, bytes.length, null);
+ Arrays.stream(clazz.getConstructors()).forEach(c -> c.setAccessible(true));
+ memo.put(name, clazz);
+ return clazz;
+ }
+ }
+}

thrift/thrift0.13/src/test/java/com/linecorp/armeria/internal/server/thrift/ThriftMetadataAccessTest.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2023 LINE Corporation
+ *
+ * LINE Corporation 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:
+ *
+ * https://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 com.linecorp.armeria.internal.server.thrift;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+
+import org.junit.jupiter.api.Test;
+
+class ThriftMetadataAccessTest {
+
+ @Test
+ void testPreInitializeTarget() {
+ final Properties properties = new Properties();
+ properties.put("armeria-thrift0.13.shortCommitHash", "c533c7fd3");
+ assertThat(ThriftMetadataAccess.needsPreInitialization(properties)).isTrue();
+
+ properties.clear();
+ properties.put("armeria-thrift0.15.shortCommitHash", "c533c7fd3");
+ assertThat(ThriftMetadataAccess.needsPreInitialization(properties)).isFalse();
+ }
+}