Skip to content

Commit

Permalink
Global utilities refactoring and enhanced interruption handling.
Browse files Browse the repository at this point in the history
1) Reuse utilities from structure module in verifier.
2) Add more interruption checks in necessary places to avoid interruption exceptions propagate to logs and messages.
  • Loading branch information
Sergey Patrikeev authored and Sergey Patrikeev committed Mar 7, 2019
1 parent 24597a7 commit 88fa83d
Show file tree
Hide file tree
Showing 68 changed files with 248 additions and 326 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jetbrains.ide.diff.builder.api

import com.jetbrains.plugin.structure.base.utils.closeAll
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.plugin.structure.classes.jdk.JdkResolverCreator
import com.jetbrains.plugin.structure.classes.resolvers.CacheResolver
import com.jetbrains.plugin.structure.classes.resolvers.Resolver
Expand Down Expand Up @@ -257,6 +258,7 @@ class IdeDiffBuilder(private val interestingPackages: List<String>, private val
return try {
findClass(className)
} catch (e: Exception) {
e.rethrowIfInterrupted()
return null
}
}
Expand Down Expand Up @@ -327,6 +329,7 @@ class IdeDiffBuilder(private val interestingPackages: List<String>, private val
null
}
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.info("Unable to read class files of a plugin $idePlugin bundled to $ide: ${e.message}")
null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.jetbrains.intellij.feature.extractor
import com.jetbrains.intellij.feature.extractor.FeaturesExtractor.extractFeatures
import com.jetbrains.intellij.feature.extractor.core.*
import com.jetbrains.plugin.structure.base.utils.closeLogged
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.plugin.structure.classes.resolvers.Resolver
import com.jetbrains.plugin.structure.classes.resolvers.UnionResolver
import com.jetbrains.plugin.structure.ide.Ide
Expand Down Expand Up @@ -46,9 +47,8 @@ object FeaturesExtractor {
ide.bundledPlugins.mapNotNull {
try {
IdePluginClassesFinder.findPluginClasses(it, additionalKeys = emptyList())
} catch (ie: InterruptedException) {
throw ie
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.error("Unable to create IDE ($ide) bundled plugin ($it) resolver", e)
null
}
Expand All @@ -74,6 +74,7 @@ object FeaturesExtractor {
try {
classNode = resolver.findClass(epImplementorClass.replace('.', '/')) ?: return null
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.debug("Unable to get plugin $plugin class file `$epImplementorClass`", e)
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ fun File.forceDeleteIfExists() {
fun File.deleteLogged(): Boolean = try {
forceDeleteIfExists()
true
} catch (ie: InterruptedException) {
Thread.currentThread().interrupt()
LOG.info("Cannot delete file because of interruption: $this")
false
} catch (e: Exception) {
LOG.error("Unable to delete $this", e)
false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,40 @@ fun checkIfInterrupted() {
}
}

/**
* Checks whether the current thread has been interrupted.
* Clears the *interrupted status*, invokes the [action],
* and throws [InterruptedException] if it is the case.
*/
@Throws(InterruptedException::class)
fun checkIfInterrupted(action: () -> Unit) {
if (Thread.interrupted()) {
try {
action()
} finally {
throw InterruptedException()
}
}
}

/**
* Throws [InterruptedException] if [this] is interrupted exception.
* Otherwise checks the current thread's interrupted flag and
* throws [InterruptedException] if it is set.
*/
fun Throwable.rethrowIfInterrupted() {
if (this is InterruptedException) {
throw this
}
checkIfInterrupted()
}

fun <T : Closeable?> T.closeLogged() {
try {
this?.close()
} catch (ie: InterruptedException) {
Thread.currentThread().interrupt()
logger.info("Cannot close because of interruption: $this")
} catch (e: Exception) {
logger.error("Unable to close $this", e)
}
Expand All @@ -36,6 +67,15 @@ inline fun <T : Closeable?, R> T.closeOnException(block: (T) -> R): R {
}
}

inline fun <T : Closeable?, R> List<T>.closeOnException(block: (List<T>) -> R): R {
try {
return block(this)
} catch (e: Throwable) {
this.forEach { t: T -> t?.closeLogged() }
throw e
}
}

/**
* Closes multiple resources and throws an exception with all failed-to-close causes
* set as suppressed exceptions, if any.
Expand All @@ -45,10 +85,14 @@ fun List<Closeable>.closeAll() {
try {
it.close()
null
} catch (ie: InterruptedException) {
Thread.currentThread().interrupt()
null
} catch (e: Exception) {
e
}
}
checkIfInterrupted()
if (exceptions.isNotEmpty()) {
val closeException = IOException("Exceptions while closing multiple resources")
exceptions.forEach { closeException.addSuppressed(it) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ fun File.extractTo(destination: File): File {
val unArchiver = createUnArchiver(this)
unArchiver.enableLogging(ConsoleLogger(Logger.LEVEL_WARN, ""))
unArchiver.destDirectory = destination
unArchiver.extract()
checkIfInterrupted()
try {
unArchiver.extract()
} catch (e: Exception) {
e.rethrowIfInterrupted()
throw e
}
return destination
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.jetbrains.plugin.structure.dotnet

import com.jetbrains.plugin.structure.base.plugin.*
import com.jetbrains.plugin.structure.base.problems.*
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.plugin.structure.dotnet.beans.extractPluginBean
import com.jetbrains.plugin.structure.dotnet.beans.toPlugin
import com.jetbrains.plugin.structure.dotnet.problems.IncorrectDotNetPluginFile
Expand Down Expand Up @@ -67,9 +68,10 @@ object ReSharperPluginManager : PluginManager<ReSharperPlugin> {
return PluginCreationSuccess(bean.toPlugin(), beanValidationResult)
} catch (e: JDOMParseException) {
val lineNumber = e.lineNumber
val message = if (lineNumber != -1) "unexpected element on line " + lineNumber else "unexpected elements"
val message = if (lineNumber != -1) "unexpected element on line $lineNumber" else "unexpected elements"
return PluginCreationFail(UnexpectedDescriptorElements(message))
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.info("Unable to read plugin descriptor from $streamName", e)
return PluginCreationFail(UnableToReadDescriptor(streamName))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.google.common.base.Joiner
import com.jetbrains.plugin.structure.base.plugin.PluginCreationFail
import com.jetbrains.plugin.structure.base.plugin.PluginCreationSuccess
import com.jetbrains.plugin.structure.base.utils.isJar
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.plugin.structure.intellij.plugin.IdePlugin
import com.jetbrains.plugin.structure.intellij.plugin.IdePluginManager.*
import com.jetbrains.plugin.structure.intellij.plugin.PluginXmlXIncludePathResolver
Expand Down Expand Up @@ -155,6 +156,7 @@ class IdeManagerImpl : IdeManager() {
}
}
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.warn("Unable to create plugin from sources: $pluginFile", e)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.jetbrains.plugin.structure.base.problems.UnableToExtractZip
import com.jetbrains.plugin.structure.base.problems.UnableToReadDescriptor
import com.jetbrains.plugin.structure.base.problems.UnexpectedDescriptorElements
import com.jetbrains.plugin.structure.base.utils.isZip
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.plugin.structure.teamcity.beans.extractPluginBean
import com.jetbrains.plugin.structure.teamcity.problems.IncorrectTeamCityPluginFile
import org.jdom2.input.JDOMParseException
Expand Down Expand Up @@ -76,12 +77,8 @@ class TeamcityPluginManager private constructor(private val validateBean: Boolea
val lineNumber = e.lineNumber
val message = if (lineNumber != -1) "unexpected element on line " + lineNumber else "unexpected elements"
return PluginCreationFail(UnexpectedDescriptorElements(message))
} catch (ie: InterruptedException) {
throw ie
} catch (e: Exception) {
if (Thread.currentThread().isInterrupted) {
throw InterruptedException()
}
e.rethrowIfInterrupted()
LOG.info("Unable to read plugin descriptor from $streamName", e)
return PluginCreationFail(UnableToReadDescriptor(DESCRIPTOR_NAME))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.jetbrains.plugins.verifier.service.server

import com.jetbrains.plugin.structure.base.utils.closeLogged
import com.jetbrains.pluginverifier.ide.IdeDescriptorsCache
import com.jetbrains.pluginverifier.ide.IdeFilesBank
import com.jetbrains.pluginverifier.ide.IdeRepository
import com.jetbrains.pluginverifier.misc.closeLogged
import com.jetbrains.pluginverifier.parameters.jdk.JdkDescriptorsCache
import com.jetbrains.pluginverifier.plugin.PluginDetailsCache
import com.jetbrains.pluginverifier.repository.repositories.marketplace.MarketplaceRepository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.jetbrains.plugins.verifier.service.server.servlets
import com.github.salomonbrys.kotson.fromJson
import com.github.salomonbrys.kotson.typeToken
import com.google.gson.Gson
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import org.jetbrains.plugins.verifier.service.server.ServerContext
import org.jetbrains.plugins.verifier.service.startup.ServerStartupListener
import org.slf4j.LoggerFactory
Expand Down Expand Up @@ -49,6 +50,7 @@ abstract class BaseServlet : HttpServlet() {
return try {
fromJson(part.inputStream)
} catch (e: Exception) {
e.rethrowIfInterrupted()
logger.error("Unable to deserialize part $partName", e)
null
}
Expand All @@ -59,6 +61,7 @@ abstract class BaseServlet : HttpServlet() {
return try {
GSON.fromJson<T>(parameter)
} catch (e: Exception) {
e.rethrowIfInterrupted()
logger.error("Unable to deserialize parameter $parameterName: $parameter", e)
null
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.plugins.verifier.service.server.servlets

import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.pluginverifier.parameters.filtering.IgnoreCondition
import org.jetbrains.plugins.verifier.service.server.servlets.info.IgnoredProblemsPage
import org.jetbrains.plugins.verifier.service.server.servlets.info.StatusPage
Expand Down Expand Up @@ -39,6 +40,7 @@ class InfoServlet : BaseServlet() {
val ignoreConditions = try {
parseIgnoreConditions(ignoredProblems)
} catch (e: Exception) {
e.rethrowIfInterrupted()
val msg = "Unable to parse ignored problems: ${e.message}"
logger.warn(msg, e)
return sendBadRequest(resp, msg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ abstract class BaseService(
logger.info("$serviceName is going to start")
doServe()
} catch (ie: InterruptedException) {
logger.info("$serviceName has been interrupted")
Thread.currentThread().interrupt()
logger.info("$serviceName has been interrupted")
} catch (e: Exception) {
logger.error("$serviceName failed to serve", e)
} finally {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.plugins.verifier.service.service.features

import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.pluginverifier.ide.IdeDescriptorsCache
import com.jetbrains.pluginverifier.ide.IdeRepository
import com.jetbrains.pluginverifier.network.NonSuccessfulResponseException
Expand Down Expand Up @@ -97,6 +98,7 @@ class FeatureExtractorService(
logger.info("Marketplace ${e.serverUrl} is currently unavailable. Stop all the scheduled updates.")
pauseFeaturesExtraction()
} catch (e: Exception) {
e.rethrowIfInterrupted()
//TODO: remove this check when the Marketplace is fixed.
if (e is NonSuccessfulResponseException && e.responseCode == 409) {
logger.info("Marketplace is still responding HTTP 409: Conflict on attempt to send update results")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.plugins.verifier.service.service.verifier

import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.pluginverifier.VerificationTarget
import com.jetbrains.pluginverifier.VerifierExecutor
import com.jetbrains.pluginverifier.ide.IdeDescriptorsCache
Expand Down Expand Up @@ -161,6 +162,7 @@ class VerifierService(
)
pauseVerification()
} catch (e: Exception) {
e.rethrowIfInterrupted()
logger.error("Unable to send verification result for $plugin", e)
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import org.jetbrains.plugins.verifier.service.tasks.Task
import org.jetbrains.plugins.verifier.service.tasks.TaskCancelledException

/**
* Task that verifies the plugin against IDE using JDK.
* Task that performs [scheduledVerification].
*/
class VerifyPluginTask(
private val scheduledVerification: ScheduledVerification,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.plugins.verifier.service.startup

import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.pluginverifier.ide.IdeDescriptorsCache
import com.jetbrains.pluginverifier.ide.IdeFilesBank
import com.jetbrains.pluginverifier.ide.ReleaseIdeRepository
Expand Down Expand Up @@ -106,6 +107,7 @@ class ServerStartupListener : ServletContextListener {
try {
return createServiceDAO(databasePath)
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.error("Unable to open/create database", e)
LOG.info("Flag to clear database on corruption is " + if (Settings.CLEAR_DATABASE_ON_CORRUPTION.getAsBoolean() == true) "ON" else "OFF")
if (Settings.CLEAR_DATABASE_ON_CORRUPTION.getAsBoolean()) {
Expand All @@ -116,6 +118,7 @@ class ServerStartupListener : ServletContextListener {
LOG.info("Successfully recreated database")
return recreatedDAO
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.error("Fatal error creating database: ${e.message}", e)
throw e
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.jetbrains.plugins.verifier.service.tasks

import com.google.common.collect.EvictingQueue
import com.google.common.util.concurrent.ThreadFactoryBuilder
import com.jetbrains.plugin.structure.base.utils.rethrowIfInterrupted
import com.jetbrains.pluginverifier.misc.shutdownAndAwaitTermination
import org.slf4j.LoggerFactory
import java.time.Instant
Expand Down Expand Up @@ -182,6 +183,7 @@ class TaskManagerImpl(private val concurrency: Int) : TaskManager {
try {
callbacks.onSuccess(result, this)
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.error("Failed 'onSuccess' callback for $this with result $result", e)
}
}
Expand All @@ -193,6 +195,7 @@ class TaskManagerImpl(private val concurrency: Int) : TaskManager {
try {
callbacks.onError(error, this)
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.error("Failed 'onError' callback for $this with error ${error.message}", e)
}
}
Expand All @@ -216,6 +219,7 @@ class TaskManagerImpl(private val concurrency: Int) : TaskManager {
try {
callbacks.onCompletion(this)
} catch (e: Exception) {
e.rethrowIfInterrupted()
LOG.error("Failed 'onCompletion' callback for $this", e)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package com.jetbrains.pluginverifier

import com.jetbrains.plugin.structure.base.utils.closeLogged
import com.jetbrains.pluginverifier.dependencies.DependenciesGraph
import com.jetbrains.pluginverifier.dependencies.presentation.DependenciesGraphPrettyPrinter
import com.jetbrains.pluginverifier.misc.buildList
import com.jetbrains.pluginverifier.misc.closeLogged
import com.jetbrains.pluginverifier.misc.replaceInvalidFileNameCharacters
import com.jetbrains.pluginverifier.output.OutputOptions
import com.jetbrains.pluginverifier.output.reporters.AllIgnoredProblemsReporter
Expand Down Expand Up @@ -186,4 +185,6 @@ class VerificationReportage(private val outputOptions: OutputOptions) : Reportag
add(FileReporter(pluginVerificationDirectory.resolve("log.txt")))
}

private inline fun <T> buildList(builderAction: MutableList<T>.() -> Unit): List<T> =
arrayListOf<T>().apply(builderAction)
}
Loading

0 comments on commit 88fa83d

Please sign in to comment.