From 6899e1f8fc7ae7462bcba1d5b57230ad840443e4 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Tue, 3 Oct 2023 22:36:20 +0530 Subject: [PATCH] fix(build-service,tooling-api): Gradle Daemon and ToolingAPI server is not stopped properly The Tooling API server (which is executed in a separate process) was not being stopped properly after IToolingApiServer.shutdown() was called. As a result, the Gradle daemon started by the tooling API was not stopped as well. This commit fixes this issue by properly implementing the shutdown() call in Tooling API. Now, when the user closes a project and the GradleBuildService is destroyed, the Tooling API server stops the Gradle daemon(s) and then shutdowns itself. --- .../services/builder/GradleBuildService.kt | 33 ++++++-- .../services/builder/ToolingServerRunner.kt | 43 +++++++---- .../services/log/MultiLogSenderHandler.kt | 1 + .../itsaky/androidide/tasks/coroutineUtils.kt | 20 ++++- .../itsaky/androidide/tooling/impl/Main.java | 19 +++++ .../tooling/impl/ToolingApiServerImpl.kt | 77 +++++++++++++++---- .../tooling/api/IToolingApiServer.kt | 18 +++-- 7 files changed, 164 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt index cc9b5af05..12e1beab3 100644 --- a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt +++ b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt @@ -43,6 +43,7 @@ import com.itsaky.androidide.projects.builder.BuildService import com.itsaky.androidide.resources.R import com.itsaky.androidide.services.ToolingServerNotStartedException import com.itsaky.androidide.services.builder.ToolingServerRunner.OnServerStartListener +import com.itsaky.androidide.tasks.ifCancelledOrInterrupted import com.itsaky.androidide.tasks.runOnUiThread import com.itsaky.androidide.tooling.api.ForwardingToolingApiClient import com.itsaky.androidide.tooling.api.IProject @@ -68,9 +69,12 @@ import kotlinx.coroutines.launch import java.io.File import java.io.IOException import java.io.InputStream +import java.io.InterruptedIOException import java.util.Objects +import java.util.concurrent.CancellationException import java.util.concurrent.CompletableFuture import java.util.concurrent.CompletionException +import java.util.concurrent.TimeUnit /** * A foreground service that handles interaction with the Gradle Tooling API. @@ -168,31 +172,36 @@ class GradleBuildService : Service(), BuildService, IToolingApiClient, } override fun onDestroy() { - mBinder!!.release() + mBinder?.release() mBinder = null + log.info("Service is being destroyed.", "Dismissing the shown notification...") notificationManager!!.cancel(NOTIFICATION_ID) + val lookup = Lookup.getDefault() lookup.unregister(BuildService.KEY_BUILD_SERVICE) lookup.unregister(BuildService.KEY_PROJECT_PROXY) server?.also { server -> - server.cancelCurrentBuild() - val shutdown = server.shutdown() try { log.info("Shutting down Tooling API server...") - shutdown.get() + // send the shutdown request but do not wait for the server to respond + // the service should not block the onDestroy call in order to avoid timeouts + // the tooling server must release resources and exit automatically + server.shutdown().get(1, TimeUnit.SECONDS) } catch (e: Throwable) { log.error("Failed to shutdown Tooling API server", e) } } + log.debug("Cancelling tooling server runner...") toolingServerRunner?.release() toolingServerRunner = null _toolingApiClient?.client = null _toolingApiClient = null + log.debug("Cancelling tooling server output reader job...") outputReaderJob?.cancel() outputReaderJob = null @@ -333,7 +342,7 @@ class GradleBuildService : Service(), BuildService, IToolingApiClient, private fun doUpdateNotification(message: String, isProgress: Boolean) { (getSystemService(NOTIFICATION_SERVICE) as NotificationManager).notify(NOTIFICATION_ID, - buildNotification(message, isProgress)) + buildNotification(message, isProgress)) } override fun initializeProject( @@ -451,8 +460,18 @@ class GradleBuildService : Service(), BuildService, IToolingApiClient, outputReaderJob = buildServiceScope.launch( Dispatchers.IO + CoroutineName("ToolingServerErrorReader")) { val reader = input.bufferedReader() - reader.forEachLine { line -> - SERVER_System_err.error(line) + try { + reader.forEachLine { line -> + SERVER_System_err.error(line) + } + } catch (e: Throwable) { + e.ifCancelledOrInterrupted(suppress = true) { + // will be suppressed + return@launch + } + + // log the error and fail silently + log.error("Failed to read tooling server output", e) } } } diff --git a/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt b/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt index 3010a2b36..545e2d1ed 100644 --- a/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt +++ b/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt @@ -19,18 +19,23 @@ package com.itsaky.androidide.services.builder import com.itsaky.androidide.shell.executeProcessAsync import com.itsaky.androidide.tasks.cancelIfActive +import com.itsaky.androidide.tasks.ifCancelledOrInterrupted import com.itsaky.androidide.tooling.api.IProject import com.itsaky.androidide.tooling.api.IToolingApiClient import com.itsaky.androidide.tooling.api.IToolingApiServer import com.itsaky.androidide.tooling.api.util.ToolingApiLauncher import com.itsaky.androidide.utils.Environment import com.itsaky.androidide.utils.ILogger +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import java.io.InputStream -import java.util.concurrent.CancellationException import java.util.concurrent.atomic.AtomicBoolean /** @@ -43,6 +48,7 @@ internal class ToolingServerRunner( private var observer: Observer?, ) { + private var _job: Job? = null private var _isStarted = AtomicBoolean(false) var isStarted: Boolean @@ -63,6 +69,7 @@ internal class ToolingServerRunner( } fun startAsync() = runnerScope.launch { + var process: Process? = null try { log.info("Starting tooling API server...") val command = listOf( @@ -87,7 +94,7 @@ internal class ToolingServerRunner( "-jar", Environment.TOOLING_API_JAR.absolutePath ) - val process = executeProcessAsync { + process = executeProcessAsync { this.command = command // input and output is used for communication to the tooling server @@ -102,7 +109,14 @@ internal class ToolingServerRunner( val errorStream = process.errorStream val processJob = launch(Dispatchers.IO) { - process.waitFor() + try { + process?.waitFor() + log.info("Tooling API process exited with code : ${process?.exitValue() ?: ""}") + process = null + } finally { + log.info("Destroying Tooling API process...") + process?.destroyForcibly() + } } val launcher = ToolingApiLauncher.newClientLauncher( @@ -132,33 +146,30 @@ internal class ToolingServerRunner( try { future.get() } catch (err: Throwable) { - when (err) { - is CancellationException, - is InterruptedException, - -> log.info("ToolingServerThread has been cancelled or interrupted.") - - else -> throw err + err.ifCancelledOrInterrupted { + log.info("ToolingServerThread has been cancelled or interrupted.") } + + // rethrow the error + throw err } } - val waitJob = launch(Dispatchers.IO) { - // wait for both, process and server job to complete - serverJob.join() - processJob.join() - } - - waitJob.join() + processJob.join() + joinAll(serverJob, processJob) } catch (e: Throwable) { if (e !is CancellationException) { log.error("Unable to start tooling API server", e) } } + }.also { + _job = it } fun release() { this.listener = null this.observer = null + this._job?.cancel(CancellationException("Cancellation was requested")) this.runnerScope.cancelIfActive("Cancellation was requested") } diff --git a/app/src/main/java/com/itsaky/androidide/services/log/MultiLogSenderHandler.kt b/app/src/main/java/com/itsaky/androidide/services/log/MultiLogSenderHandler.kt index 195d0b40c..5e3ef345e 100644 --- a/app/src/main/java/com/itsaky/androidide/services/log/MultiLogSenderHandler.kt +++ b/app/src/main/java/com/itsaky/androidide/services/log/MultiLogSenderHandler.kt @@ -30,6 +30,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import java.lang.Thread.currentThread import java.net.ServerSocket +import java.util.concurrent.CancellationException import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger diff --git a/common/src/main/java/com/itsaky/androidide/tasks/coroutineUtils.kt b/common/src/main/java/com/itsaky/androidide/tasks/coroutineUtils.kt index 05e49bb09..f5fc0502a 100644 --- a/common/src/main/java/com/itsaky/androidide/tasks/coroutineUtils.kt +++ b/common/src/main/java/com/itsaky/androidide/tasks/coroutineUtils.kt @@ -21,6 +21,7 @@ import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.cancel +import java.io.InterruptedIOException /** * Calls [CoroutineScope.cancel] only if a job is active in the scope. @@ -39,7 +40,22 @@ fun CoroutineScope.cancelIfActive(message: String, cause: Throwable? = null) = */ fun CoroutineScope.cancelIfActive(exception: CancellationException? = null) { val job = coroutineContext[Job] - if (job?.isActive == true) { - cancel(exception) + job?.cancel(exception) +} + +/** + * Runs the given [action] and re-throws the [Throwable] if this [Throwable] is thrown for + * cancellation or interruption purposes. + * + * @param action The action to run if this [Throwable] was cancelled or interrupted. + */ +inline fun Throwable.ifCancelledOrInterrupted(suppress: Boolean = false, action: () -> Unit) { + when (this) { + is CancellationException, + is InterruptedException, + is InterruptedIOException -> { + action() + if (!suppress) { throw this } + } } } \ No newline at end of file diff --git a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/Main.java b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/Main.java index 993c70ff0..04aa18c40 100644 --- a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/Main.java +++ b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/Main.java @@ -31,6 +31,7 @@ import java.nio.charset.StandardCharsets; import java.util.HashSet; import java.util.Objects; import java.util.Set; +import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import org.gradle.tooling.ConfigurableLauncher; @@ -59,8 +60,26 @@ public class Main { LOG.debug("Server started. Will run until shutdown message is received..."); try { Main.future.get(); + } catch (CancellationException cancellationException) { + // ignored } catch (InterruptedException | ExecutionException e) { LOG.error("An error occurred while waiting for shutdown message", e); + } finally { + + // Cleanup should be performed in ToolingApiServerImpl.shutdown() + // this is to make sure that the daemons are stopped in case the client doesn't call shutdown() + try { + if (server.isInitialized() || server.isConnected()) { + LOG.warn("Connection to tooling server closed without shutting it down!"); + server.shutdown().get(); + } + } catch (InterruptedException | ExecutionException e) { + e.printStackTrace(); + } finally { + future = null; + client = null; + JvmLogger.interceptor = null; + } } } diff --git a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt index 85b1f237d..6554bf5b0 100644 --- a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt +++ b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt @@ -50,12 +50,17 @@ import org.gradle.tooling.CancellationTokenSource import org.gradle.tooling.GradleConnectionException import org.gradle.tooling.GradleConnector import org.gradle.tooling.ProjectConnection +import org.gradle.tooling.ResultHandler import org.gradle.tooling.UnsupportedVersionException import org.gradle.tooling.exceptions.UnsupportedBuildArgumentException import org.gradle.tooling.exceptions.UnsupportedOperationConfigurationException +import org.gradle.tooling.internal.consumer.BlockingResultHandler +import org.gradle.tooling.internal.consumer.DefaultGradleConnector import java.io.File import java.util.concurrent.CompletableFuture import java.util.concurrent.CompletionException +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.system.exitProcess /** * Implementation for the Gradle Tooling API server. @@ -65,7 +70,7 @@ import java.util.concurrent.CompletionException internal class ToolingApiServerImpl(private val project: ProjectImpl) : IToolingApiServer { - private var initialized = false + private val _isInitialized = AtomicBoolean(false) private var client: IToolingApiClient? = null private var connector: GradleConnector? = null private var connection: ProjectConnection? = null @@ -73,6 +78,31 @@ internal class ToolingApiServerImpl(private val project: ProjectImpl) : private var buildCancellationToken: CancellationTokenSource? = null private val log = ILogger.newInstance(javaClass.simpleName) + /** + * Whether the project has been initialized or not. + */ + var isInitialized: Boolean + get() = _isInitialized.get() + private set(value) = _isInitialized.set(value) + + /** + * Whether the server has a live connection to Gradle. + */ + val isConnected: Boolean + get() = connector != null || connection != null + + companion object { + + /** + * Time duration for which the the Tooling API server waits after calling + * [DefaultGradleConnector.close] and before exiting the server's process. + * + * This delay should be long enough to let the tooling API stop the daemon but short enough so + * that the server's process is not kept alive for longer duration. + */ + const val DELAY_BEFORE_EXIT_MS = 1000L + } + override fun initialize(params: InitializeProjectParams): CompletableFuture { return CompletableFuture.supplyAsync { try { @@ -133,7 +163,7 @@ internal class ToolingApiServerImpl(private val project: ProjectImpl) : this.project.rootProject = project.rootProject this.project.projects = project.projects - initialized = true + this.isInitialized = true notifyBuildSuccess(emptyList()) return@supplyAsync InitializeResult(true) @@ -146,7 +176,7 @@ internal class ToolingApiServerImpl(private val project: ProjectImpl) : } override fun isServerInitialized(): CompletableFuture { - return CompletableFuture.supplyAsync { initialized } + return CompletableFuture.supplyAsync { isInitialized } } override fun getRootProject(): CompletableFuture { @@ -257,21 +287,38 @@ internal class ToolingApiServerImpl(private val project: ProjectImpl) : } override fun shutdown(): CompletableFuture { - return CompletableFuture.runAsync { - connection?.close() - connector?.disconnect() - Main.future?.cancel(true) + log.info("Shutting down Tooling API Server...") - this.connection = null - this.connector = null - this.client = null - this.buildCancellationToken = null - this.lastInitParams = null - Main.future = null - Main.client = null + connection?.close() + connector?.disconnect() + connection = null + connector = null - this.initialized = false + // Stop all daemons + log.info("Stopping all Gradle Daemons...") + DefaultGradleConnector.close() + + try { + // wait for the daemon to be stopped properly + Thread.sleep(DELAY_BEFORE_EXIT_MS) + } catch (e: Exception) { + // ignored } + + // update the initialization flag before cancelling future + this.isInitialized = false + + Main.future?.cancel(true) + + this.client = null + this.buildCancellationToken = null // connector.disconnect() cancels any running builds + this.lastInitParams = null + Main.future = null + Main.client = null + + // no need to return anything + // tooling server should be restarted once it has been shutdown + exitProcess(0) } private fun getTaskFailureType(error: Throwable): Failure = diff --git a/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiServer.kt b/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiServer.kt index 7ad94ff7d..c653ee00f 100644 --- a/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiServer.kt +++ b/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiServer.kt @@ -22,7 +22,6 @@ import com.itsaky.androidide.tooling.api.messages.TaskExecutionMessage import com.itsaky.androidide.tooling.api.messages.result.BuildCancellationRequestResult import com.itsaky.androidide.tooling.api.messages.result.InitializeResult import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult -import org.eclipse.lsp4j.jsonrpc.services.JsonNotification import org.eclipse.lsp4j.jsonrpc.services.JsonRequest import org.eclipse.lsp4j.jsonrpc.services.JsonSegment import java.util.concurrent.* @@ -36,13 +35,16 @@ import java.util.concurrent.* interface IToolingApiServer { /** Initialize the server with the project directory. */ - @JsonRequest fun initialize(params: InitializeProjectParams): CompletableFuture + @JsonRequest + fun initialize(params: InitializeProjectParams): CompletableFuture /** Is the server initialized? */ - @JsonRequest fun isServerInitialized(): CompletableFuture + @JsonRequest + fun isServerInitialized(): CompletableFuture /** Get the root project. */ - @JsonRequest fun getRootProject(): CompletableFuture + @JsonRequest + fun getRootProject(): CompletableFuture /** Execute the tasks specified in the message. */ @JsonRequest @@ -54,12 +56,14 @@ interface IToolingApiServer { * @return A [CompletableFuture] which completes when the current build cancellation process * finishes (either successfully or with an error). */ - @JsonRequest fun cancelCurrentBuild(): CompletableFuture + @JsonRequest + fun cancelCurrentBuild(): CompletableFuture /** - * Shutdown the tooling API server. This will disconnect all the project connection instances.] + * Shutdown the tooling API server. This will disconnect all the project connection instances. * * @return A [CompletableFuture] which completes when the shutdown process is finished. */ - @JsonNotification fun shutdown(): CompletableFuture + @JsonRequest + fun shutdown(): CompletableFuture }