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

… service (#4688)

Motivation:

We use thrift/grpc internal APIs when constructing `DocService`.
Following #4491, these internal APIs are called concurrently with service logic.
Due to a thrift bug, a deadlock can occur when `FieldMetaData.getStructMetaDataMap` is called for a thrift class while the class is initialized.
https://issues.apache.org/jira/browse/THRIFT-5430
This PR attempts to circumvent this issue by pre-loading the class for affected versions (<= armeria-thrift 0.14)

I've also double checked that other `*MetaData` implementations don't have a similar implementation.

Modifications:

- Add a `ThriftMetadataAccess` utility class
- When `ThriftMetadataAccess` is instantiated, check if (<= armeria-thrift 0.14) is in the classpath
- If the vulnerable artifact is loaded, then pre-instantiate thrift classes before `FieldMetaData.getStructMetaDataMap` is called from `DocService`

Result:

- A deadlock is no longer observed when loading `DocService` for thrift services where `<= armeria-thrift 0.14`

thrift/thrift0.12/build.gradle

@@ -26,6 +26,8 @@ dependencies {
  testImplementation libs.prometheus
 }
 
+tasks.processResources.from "${rootProject.projectDir}/thrift/thrift0.13/src/main/resources"
+
 // Use the test sources from ':thrift0.13'.
 // NB: We should never add these directories using the 'sourceSets' directive because that will make
 // them added to more than one project and having a source directory with more than one output directory

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,83 @@
+/*
+ * 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.Map;
+import java.util.Properties;
+
+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 String THRIFT_OPTIONS_PROPERTIES = "../../common/thrift/thrift-options.properties";
+
+ static {
+ try {
+ final URL url = ThriftMetadataAccess.class.getResource(THRIFT_OPTIONS_PROPERTIES);
+ if (url != null) {
+ try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream()))) {
+ final Properties props = new Properties();
+ props.load(reader);
+ preInitializeThriftClass = needsPreInitialization(props);
+ }
+ } else {
+ preInitializeThriftClass = true;
+ }
+ } catch (Exception e) {
+ logger.debug("Unexpected exception while extracting options: ", e);
+ preInitializeThriftClass = true;
+ }
+ }
+
+ @VisibleForTesting
+ static boolean needsPreInitialization(Properties properties) {
+ return "true".equals(properties.getProperty("struct.preinit"));
+ }
+
+ @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/main/resources/com/linecorp/armeria/internal/common/thrift/thrift-options.properties

@@ -0,0 +1 @@
+struct.preinit=true

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 < 50; 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,39 @@
+/*
+ * 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 basicCase() {
+ final Properties props = new Properties();
+ props.put("struct.preinit", "true");
+ assertThat(ThriftMetadataAccess.needsPreInitialization(props)).isTrue();
+ }
+
+ @Test
+ void failingCase() {
+ final Properties props = new Properties();
+ assertThat(ThriftMetadataAccess.needsPreInitialization(props)).isFalse();
+ }
+}

thrift/thrift0.15/build.gradle

@@ -54,7 +54,9 @@ tasks.generateSources.dependsOn(generateTestSources)
 tasks.compileJava.dependsOn(generateSources)
 tasks.compileTestJava.dependsOn(generateSources)
 
-tasks.processResources.from "${thrift013ProjectDir}/src/main/resources"
+tasks.processResources.from("${thrift013ProjectDir}/src/main/resources") {
+ exclude '**/thrift-options.properties'
+}
 tasks.processTestResources.from "${thrift013ProjectDir}/src/test/resources"
 tasks.sourcesJar.from "${thrift013ProjectDir}/src/main/resources"
 

thrift/thrift0.16/build.gradle

@@ -55,7 +55,9 @@ tasks.generateSources.dependsOn(generateTestSources)
 tasks.compileJava.dependsOn(generateSources)
 tasks.compileTestJava.dependsOn(generateSources)
 
-tasks.processResources.from "${thrift013ProjectDir}/src/main/resources"
+tasks.processResources.from("${thrift013ProjectDir}/src/main/resources") {
+ exclude '**/thrift-options.properties'
+}
 tasks.processTestResources.from "${thrift013ProjectDir}/src/test/resources"
 tasks.sourcesJar.from "${thrift013ProjectDir}/src/main/resources"
 

thrift/thrift0.17/build.gradle

@@ -56,7 +56,9 @@ tasks.generateSources.dependsOn(generateTestSources)
 tasks.compileJava.dependsOn(generateSources)
 tasks.compileTestJava.dependsOn(generateSources)
 
-tasks.processResources.from "${thrift013ProjectDir}/src/main/resources"
+tasks.processResources.from("${thrift013ProjectDir}/src/main/resources") {
+ exclude '**/thrift-options.properties'
+}
 tasks.processTestResources.from "${thrift013ProjectDir}/src/test/resources"
 tasks.sourcesJar.from "${thrift013ProjectDir}/src/main/resources"