Fix part of #5312, part of #59: Introduce better script execution support

scripts/assets/maven_dependencies.textproto

@@ -1050,8 +1050,19 @@ maven_dependency {
   }
 }
 maven_dependency {
-  artifact_name: "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.4.1"
-  artifact_version: "1.4.1"
+  artifact_name: "org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.4.3"
+  artifact_version: "1.4.3"
+  license {
+    license_name: "The Apache Software License, Version 2.0"
+    original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt"
+    scrapable_link {
+      url: "https://www.apache.org/licenses/LICENSE-2.0.txt"
+    }
+  }
+}
+maven_dependency {
+  artifact_name: "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.4.3"
+  artifact_version: "1.4.3"
   license {
     license_name: "The Apache Software License, Version 2.0"
     original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt"

scripts/src/java/org/oppia/android/scripts/build/TransformAndroidManifest.kt

@@ -1,6 +1,8 @@
 package org.oppia.android.scripts.build
 
+import org.oppia.android.scripts.common.CommandExecutorImpl
 import org.oppia.android.scripts.common.GitClient
+import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher
 import org.w3c.dom.Document
 import org.w3c.dom.NodeList
 import java.io.File
@@ -62,21 +64,24 @@ fun main(args: Array<String>) {
   check(args.size >= 9) { USAGE_STRING }
 
   val repoRoot = File(args[0]).also { if (!it.exists()) error("File doesn't exist: ${args[0]}") }
-  TransformAndroidManifest(
-    repoRoot = repoRoot,
-    sourceManifestFile = File(args[1]).also {
-      if (!it.exists()) {
-        error("File doesn't exist: ${args[1]}")
-      }
-    },
-    outputManifestFile = File(args[2]),
-    buildFlavor = args[3],
-    majorVersion = args[4].toIntOrNull() ?: error(USAGE_STRING),
-    minorVersion = args[5].toIntOrNull() ?: error(USAGE_STRING),
-    versionCode = args[6].toIntOrNull() ?: error(USAGE_STRING),
-    relativelyQualifiedApplicationClass = args[7],
-    baseDevelopBranchReference = args[8]
-  ).generateAndOutputNewManifest()
+  ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher ->
+    TransformAndroidManifest(
+      repoRoot = repoRoot,
+      sourceManifestFile = File(args[1]).also {
+        if (!it.exists()) {
+          error("File doesn't exist: ${args[1]}")
+        }
+      },
+      outputManifestFile = File(args[2]),
+      buildFlavor = args[3],
+      majorVersion = args[4].toIntOrNull() ?: error(USAGE_STRING),
+      minorVersion = args[5].toIntOrNull() ?: error(USAGE_STRING),
+      versionCode = args[6].toIntOrNull() ?: error(USAGE_STRING),
+      relativelyQualifiedApplicationClass = args[7],
+      baseDevelopBranchReference = args[8],
+      scriptBgDispatcher
+    ).generateAndOutputNewManifest()
+  }
 }
 
 private class TransformAndroidManifest(
@@ -88,11 +93,11 @@ private class TransformAndroidManifest(
   private val minorVersion: Int,
   private val versionCode: Int,
   private val relativelyQualifiedApplicationClass: String,
-  private val baseDevelopBranchReference: String
+  private val baseDevelopBranchReference: String,
+  private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher
 ) {
-  private val gitClient by lazy {
-    GitClient(repoRoot, baseDevelopBranchReference)
-  }
+  private val commandExecutor by lazy { CommandExecutorImpl(scriptBgDispatcher) }
+  private val gitClient by lazy { GitClient(repoRoot, baseDevelopBranchReference, commandExecutor) }
   private val documentBuilderFactory by lazy { DocumentBuilderFactory.newInstance() }
   private val transformerFactory by lazy { TransformerFactory.newInstance() }
 

scripts/src/java/org/oppia/android/scripts/ci/ComputeAffectedTests.kt

@@ -5,6 +5,7 @@ import org.oppia.android.scripts.common.CommandExecutor
 import org.oppia.android.scripts.common.CommandExecutorImpl
 import org.oppia.android.scripts.common.GitClient
 import org.oppia.android.scripts.common.ProtoStringEncoder.Companion.toCompressedBase64
+import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher
 import org.oppia.android.scripts.proto.AffectedTestsBucket
 import java.io.File
 import java.util.Locale
@@ -59,9 +60,10 @@ fun main(args: Array<String>) {
           " '$computeAllTestsValue'"
       )
   }
-  ComputeAffectedTests().compute(
-    pathToRoot, pathToOutputFile, baseDevelopBranchReference, computeAllTestsSetting
-  )
+  ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher ->
+    ComputeAffectedTests(scriptBgDispatcher)
+      .compute(pathToRoot, pathToOutputFile, baseDevelopBranchReference, computeAllTestsSetting)
+  }
 }
 
 // Needed since the codebase isn't yet using Kotlin 1.5, so this function isn't available.
@@ -75,10 +77,11 @@ private fun String.toBooleanStrictOrNull(): Boolean? {
 
 /** Utility used to compute affected test targets. */
 class ComputeAffectedTests(
-  private val maxTestCountPerLargeShard: Int = MAX_TEST_COUNT_PER_LARGE_SHARD,
-  private val maxTestCountPerMediumShard: Int = MAX_TEST_COUNT_PER_MEDIUM_SHARD,
-  private val maxTestCountPerSmallShard: Int = MAX_TEST_COUNT_PER_SMALL_SHARD,
-  private val commandExecutor: CommandExecutor = CommandExecutorImpl()
+  private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher,
+  val maxTestCountPerLargeShard: Int = MAX_TEST_COUNT_PER_LARGE_SHARD,
+  val maxTestCountPerMediumShard: Int = MAX_TEST_COUNT_PER_MEDIUM_SHARD,
+  val maxTestCountPerSmallShard: Int = MAX_TEST_COUNT_PER_SMALL_SHARD,
+  val commandExecutor: CommandExecutor = CommandExecutorImpl(scriptBgDispatcher)
 ) {
   private companion object {
     private const val GENERIC_TEST_BUCKET_NAME = "generic"
@@ -108,7 +111,7 @@ class ComputeAffectedTests(
 
     println("Running from directory root: $rootDirectory")
 
-    val gitClient = GitClient(rootDirectory, baseDevelopBranchReference)
+    val gitClient = GitClient(rootDirectory, baseDevelopBranchReference, commandExecutor)
     val bazelClient = BazelClient(rootDirectory, commandExecutor)
     println("Current branch: ${gitClient.currentBranch}")
     println("Most recent common commit: ${gitClient.branchMergeBase}")

scripts/src/java/org/oppia/android/scripts/common/BUILD.bazel

@@ -36,6 +36,10 @@ kt_jvm_library(
         "CommandResult.kt",
     ],
     visibility = ["//scripts:oppia_script_library_visibility"],
+    deps = [
+        ":script_background_coroutine_dispatcher",
+        "//third_party:org_jetbrains_kotlinx_kotlinx-coroutines-core",
+    ],
 )
 
 kt_jvm_library(
@@ -57,3 +61,12 @@ kt_jvm_library(
         "//third_party:org_jetbrains_kotlin_kotlin-stdlib-jdk8_jar",
     ],
 )
+
+kt_jvm_library(
+    name = "script_background_coroutine_dispatcher",
+    srcs = ["ScriptBackgroundCoroutineDispatcher.kt"],
+    visibility = ["//scripts:oppia_script_library_visibility"],
+    deps = [
+        "//third_party:org_jetbrains_kotlinx_kotlinx-coroutines-core",
+    ],
+)

scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt

@@ -10,7 +10,7 @@ import java.util.Locale
  */
 class BazelClient(
   private val rootDirectory: File,
-  private val commandExecutor: CommandExecutor = CommandExecutorImpl()
+  private val commandExecutor: CommandExecutor
 ) {
   /** Returns all Bazel test targets in the workspace. */
   fun retrieveAllTestTargets(): List<String> {

scripts/src/java/org/oppia/android/scripts/common/CommandExecutorImpl.kt

@@ -1,5 +1,8 @@
 package org.oppia.android.scripts.common
 
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.async
+import kotlinx.coroutines.runBlocking
 import java.io.File
 import java.util.concurrent.TimeUnit
 
@@ -11,6 +14,7 @@ const val WAIT_PROCESS_TIMEOUT_MS = 60_000L
 
 /** Default implementation of [CommandExecutor]. */
 class CommandExecutorImpl(
+  private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher,
   private val processTimeout: Long = WAIT_PROCESS_TIMEOUT_MS,
   private val processTimeoutUnit: TimeUnit = TimeUnit.MILLISECONDS
 ) : CommandExecutor {
@@ -29,7 +33,11 @@ class CommandExecutorImpl(
         .directory(workingDir)
         .redirectErrorStream(includeErrorOutput)
         .start()
-    val finished = process.waitFor(processTimeout, processTimeoutUnit)
+    val finished = runBlocking {
+      CoroutineScope(scriptBgDispatcher).async {
+        process.waitFor(processTimeout, processTimeoutUnit)
+      }.await()
+    }
     check(finished) { "Process did not finish within the expected timeout" }
     return CommandResult(
       process.exitValue(),

scripts/src/java/org/oppia/android/scripts/common/GitClient.kt

@@ -9,10 +9,9 @@ import java.io.File
  */
 class GitClient(
   private val workingDirectory: File,
-  private val baseDevelopBranchReference: String
+  private val baseDevelopBranchReference: String,
+  private val commandExecutor: CommandExecutor
 ) {
-  private val commandExecutor by lazy { CommandExecutorImpl() }
-
   /** The commit hash of the HEAD of the local Git repository. */
   val currentCommit: String by lazy { retrieveCurrentCommit() }
 

scripts/src/java/org/oppia/android/scripts/common/ScriptBackgroundCoroutineDispatcher.kt

@@ -0,0 +1,86 @@
+package org.oppia.android.scripts.common
+
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.Runnable
+import kotlinx.coroutines.asCoroutineDispatcher
+import java.io.Closeable
+import java.util.concurrent.ExecutorService
+import java.util.concurrent.Executors
+import java.util.concurrent.TimeUnit
+import kotlin.coroutines.CoroutineContext
+
+/**
+ * A [CoroutineDispatcher] that's [Closeable] and particularly tailored to be easily used in scripts
+ * that need to perform parallel tasks for expensive IO. It's highly recommended to exclusively use
+ * this dispatcher over any others, and to ensure that [close] is called at the end of the script to
+ * avoid any potential threads hanging (causing the script to not actually close).
+ *
+ * Note that the dispatcher attempts to finish any ongoing tasks when [close] is called, but it will
+ * reject new tasks from being scheduled and it will force terminate if any pending tasks at the
+ * time of closing don't end within the configured [closeTimeout] provided.
+ *
+ * A simple example for using this dispatcher:
+ * ```kotlin
+ * ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher ->
+ *   val deferred = CoroutineScope(scriptBgDispatcher).async {
+ *     // Expensive task...
+ *   }
+ *   // IMPORTANT: The operation must be observed before use{} ends, otherwise the dispatcher will
+ *   // close and terminate any pending tasks.
+ *   runBlocking { deferred.await() }
+ * }
+ * ```
+ *
+ * A more complex example for I/O operations:
+ * ```kotlin
+ * ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher ->
+ *   val deferred = CoroutineScope(scriptBgDispatcher).async {
+ *     withContext(Dispatchers.IO) {
+ *      // Perform I/O using Kotlin's highly parallelized I/O dispatcher, but wait for the result
+ *      // using the background script dispatcher (since execution could continue if other I/O
+ *      // operations need to be kicked off, or if other work can be done alongside the I/O).
+ *     }
+ *   }
+ *   // IMPORTANT: The operation must be observed before use{} ends, otherwise the dispatcher will
+ *   // close and terminate any pending tasks.
+ *   runBlocking { deferred.await() }
+ * }
+ * ```
+ *
+ * @property closeTimeout the amount of time, in [closeTimeoutUnit] units, that should be waited
+ *     when [close]ing this dispatcher before force-ending ongoing tasks
+ * @property closeTimeoutUnit the unit of time used for [closeTimeout]
+ */
+class ScriptBackgroundCoroutineDispatcher(
+  private val closeTimeout: Long = 5,
+  private val closeTimeoutUnit: TimeUnit = TimeUnit.SECONDS
+) : CoroutineDispatcher(), Closeable {
+  private val threadPool by lazy { Executors.newCachedThreadPool() }
+  private val coroutineDispatcher by lazy { threadPool.asCoroutineDispatcher() }
+
+  override fun dispatch(context: CoroutineContext, block: Runnable) {
+    coroutineDispatcher.dispatch(context, block)
+  }
+
+  override fun close() {
+    threadPool.tryShutdownFully(timeout = closeTimeout, unit = closeTimeoutUnit)
+    coroutineDispatcher.close()
+  }
+
+  private companion object {
+    private fun ExecutorService.tryShutdownFully(timeout: Long, unit: TimeUnit) {
+      // Try to fully shutdown the executor service per https://stackoverflow.com/a/33690603 and
+      // https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ExecutorService.html.
+      shutdown()
+      try {
+        if (!awaitTermination(timeout, unit)) {
+          shutdownNow()
+          check(awaitTermination(timeout, unit)) { "ExecutorService didn't fully shutdown: $this." }
+        }
+      } catch (e: InterruptedException) {
+        shutdownNow()
+        Thread.currentThread().interrupt()
+      }
+    }
+  }
+}

scripts/src/java/org/oppia/android/scripts/license/MavenDependenciesListCheck.kt

@@ -3,6 +3,7 @@ package org.oppia.android.scripts.license
 import com.google.protobuf.TextFormat
 import org.oppia.android.scripts.common.CommandExecutor
 import org.oppia.android.scripts.common.CommandExecutorImpl
+import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher
 import org.oppia.android.scripts.proto.MavenDependency
 
 /**
@@ -24,7 +25,9 @@ import org.oppia.android.scripts.proto.MavenDependency
  *   third_party/maven_install.json scripts/assets/maven_dependencies.pb
  */
 fun main(args: Array<String>) {
-  MavenDependenciesListCheck(LicenseFetcherImpl()).main(args)
+  ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher ->
+    MavenDependenciesListCheck(LicenseFetcherImpl(), scriptBgDispatcher).main(args)
+  }
 }
 
 /**
@@ -33,7 +36,8 @@ fun main(args: Array<String>) {
  */
 class MavenDependenciesListCheck(
   private val licenseFetcher: LicenseFetcher,
-  private val commandExecutor: CommandExecutor = CommandExecutorImpl()
+  scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher,
+  private val commandExecutor: CommandExecutor = CommandExecutorImpl(scriptBgDispatcher)
 ) {
 
   /**

scripts/src/java/org/oppia/android/scripts/license/MavenDependenciesRetriever.kt

@@ -5,7 +5,6 @@ import com.squareup.moshi.Moshi
 import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
 import org.oppia.android.scripts.common.BazelClient
 import org.oppia.android.scripts.common.CommandExecutor
-import org.oppia.android.scripts.common.CommandExecutorImpl
 import org.oppia.android.scripts.maven.model.MavenListDependency
 import org.oppia.android.scripts.maven.model.MavenListDependencyTree
 import org.oppia.android.scripts.proto.License
@@ -24,7 +23,7 @@ private const val MAVEN_PREFIX = "@maven//:"
 class MavenDependenciesRetriever(
   private val rootPath: String,
   private val licenseFetcher: LicenseFetcher,
-  private val commandExecutor: CommandExecutor = CommandExecutorImpl()
+  private val commandExecutor: CommandExecutor
 ) {
 
   private val bazelClient by lazy {