Skip to content

Commit 6f0dfdb

Browse files
bell-dbbell-db
committed
Handle all failures in Future
1 parent 3041aed commit 6f0dfdb

File tree

5 files changed

+91
-75
lines changed

5 files changed

+91
-75
lines changed

‎bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala

+10-8
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,7 @@ object PluginFrontend {
4747
gen: ProtocCodeGenerator,
4848
request: Array[Byte]
4949
): Array[Byte] = {
50-
Try {
51-
gen.run(request)
52-
}.recover { case throwable =>
53-
createCodeGeneratorResponseWithError(
54-
throwable.toString + "\n" + getStackTrace(throwable)
55-
)
56-
}.get
50+
gen.run(request)
5751
}
5852

5953
def createCodeGeneratorResponseWithError(error: String): Array[Byte] = {
@@ -116,9 +110,17 @@ object PluginFrontend {
116110
gen: ProtocCodeGenerator,
117111
fsin: InputStream,
118112
env: ExtraEnv
119-
): Array[Byte] = {
113+
): Array[Byte] = try {
120114
val bytes = readInputStreamToByteArrayWithEnv(fsin, env)
121115
runWithBytes(gen, bytes)
116+
} catch {
117+
// This covers all Throwable including OutOfMemoryError, StackOverflowError, etc.
118+
// We need to make a best effort to return a response to protoc,
119+
// otherwise protoc can hang indefinitely.
120+
case throwable: Throwable =>
121+
createCodeGeneratorResponseWithError(
122+
throwable.toString + "\n" + getStackTrace(throwable)
123+
)
122124
}
123125

124126
def createTempFile(extension: String, content: String): Path = {

‎bridge/src/main/scala/protocbridge/frontend/PosixPluginFrontend.scala

+3
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ object PosixPluginFrontend extends PluginFrontend {
4040
val response = PluginFrontend.runWithInputStream(plugin, fsin, env)
4141
fsin.close()
4242

43+
// Note that the output pipe must be opened after the input pipe is closed.
44+
// Otherwise, there might be a deadlock if the shell script is stuck in input pipe and
45+
// doesn't open the other end of the output pipe.
4346
val fsout = Files.newOutputStream(outputPipe)
4447
fsout.write(response)
4548
fsout.close()

‎bridge/src/test/scala/protocbridge/frontend/OsSpecificFrontendSpec.scala

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package protocbridge.frontend
2+
3+
import org.apache.commons.io.IOUtils
4+
import org.scalatest.flatspec.AnyFlatSpec
5+
import org.scalatest.matchers.must.Matchers
6+
import protocbridge.{ExtraEnv, ProtocCodeGenerator}
7+
8+
import java.io.ByteArrayOutputStream
9+
import scala.sys.process.ProcessIO
10+
import scala.util.Random
11+
12+
class OsSpecificFrontendSpec extends AnyFlatSpec with Matchers {
13+
14+
protected def testPluginFrontend(
15+
frontend: PluginFrontend,
16+
generator: ProtocCodeGenerator,
17+
env: ExtraEnv,
18+
request: Array[Byte]
19+
): Array[Byte] = {
20+
val (path, state) = frontend.prepare(
21+
generator,
22+
env
23+
)
24+
val actualOutput = new ByteArrayOutputStream()
25+
val process = sys.process
26+
.Process(path.toAbsolutePath.toString)
27+
.run(new ProcessIO(writeInput => {
28+
writeInput.write(request)
29+
writeInput.close()
30+
}, processOutput => {
31+
IOUtils.copy(processOutput, actualOutput)
32+
processOutput.close()
33+
}, _.close()))
34+
process.exitValue()
35+
frontend.cleanup(state)
36+
actualOutput.toByteArray
37+
}
38+
39+
protected def testSuccess(frontend: PluginFrontend): Unit = {
40+
val random = new Random()
41+
val toSend = Array.fill(123)(random.nextInt(256).toByte)
42+
val toReceive = Array.fill(456)(random.nextInt(256).toByte)
43+
val env = new ExtraEnv(secondaryOutputDir = "tmp")
44+
45+
val fakeGenerator = new ProtocCodeGenerator {
46+
override def run(request: Array[Byte]): Array[Byte] = {
47+
request mustBe (toSend ++ env.toByteArrayAsField)
48+
toReceive
49+
}
50+
}
51+
val response = testPluginFrontend(frontend, fakeGenerator, env, toSend)
52+
response mustBe toReceive
53+
}
54+
55+
protected def testFailure(frontend: PluginFrontend): Unit = {
56+
val random = new Random()
57+
val toSend = Array.fill(123)(random.nextInt(256).toByte)
58+
val env = new ExtraEnv(secondaryOutputDir = "tmp")
59+
60+
val fakeGenerator = new ProtocCodeGenerator {
61+
override def run(request: Array[Byte]): Array[Byte] = {
62+
throw new OutOfMemoryError("test error")
63+
}
64+
}
65+
val response = testPluginFrontend(frontend, fakeGenerator, env, toSend)
66+
response.length must be > 0
67+
}
68+
}

‎bridge/src/test/scala/protocbridge/frontend/PosixPluginFrontendSpec.scala

+5-37
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,13 @@
11
package protocbridge.frontend
22

3-
import org.apache.commons.io.IOUtils
4-
import org.scalatest.flatspec.AnyFlatSpec
5-
import org.scalatest.matchers.must.Matchers
6-
import protocbridge.{ExtraEnv, ProtocCodeGenerator}
7-
8-
import java.io.ByteArrayOutputStream
9-
import scala.sys.process.ProcessIO
10-
import scala.util.Random
11-
12-
class PosixPluginFrontendSpec extends AnyFlatSpec with Matchers {
3+
class PosixPluginFrontendSpec extends OsSpecificFrontendSpec {
134
if (!PluginFrontend.isWindows) {
145
it must "execute a program that forwards input and output to given stream" in {
15-
val random = new Random()
16-
val toSend = Array.fill(123)(random.nextInt(256).toByte)
17-
val toReceive = Array.fill(456)(random.nextInt(256).toByte)
18-
val env = new ExtraEnv(secondaryOutputDir = "tmp")
6+
testSuccess(PosixPluginFrontend)
7+
}
198

20-
val fakeGenerator = new ProtocCodeGenerator {
21-
override def run(request: Array[Byte]): Array[Byte] = {
22-
request mustBe (toSend ++ env.toByteArrayAsField)
23-
toReceive
24-
}
25-
}
26-
val (path, state) = PosixPluginFrontend.prepare(
27-
fakeGenerator,
28-
env
29-
)
30-
val actualOutput = new ByteArrayOutputStream()
31-
val process = sys.process
32-
.Process(path.toAbsolutePath.toString)
33-
.run(new ProcessIO(writeInput => {
34-
writeInput.write(toSend)
35-
writeInput.close()
36-
}, processOutput => {
37-
IOUtils.copy(processOutput, actualOutput)
38-
processOutput.close()
39-
}, _.close()))
40-
process.exitValue()
41-
actualOutput.toByteArray mustBe toReceive
42-
PosixPluginFrontend.cleanup(state)
9+
it must "not hang if there is an error in generator" in {
10+
testFailure(PosixPluginFrontend)
4311
}
4412
}
4513
}

‎bridge/src/test/scala/protocbridge/frontend/WindowsPluginFrontendSpec.scala

+5-30
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,13 @@
11
package protocbridge.frontend
22

3-
import java.io.ByteArrayInputStream
4-
5-
import protocbridge.{ProtocCodeGenerator, ExtraEnv}
6-
7-
import scala.sys.process.ProcessLogger
8-
import org.scalatest.flatspec.AnyFlatSpec
9-
import org.scalatest.matchers.must.Matchers
10-
11-
class WindowsPluginFrontendSpec extends AnyFlatSpec with Matchers {
3+
class WindowsPluginFrontendSpec extends OsSpecificFrontendSpec {
124
if (PluginFrontend.isWindows) {
135
it must "execute a program that forwards input and output to given stream" in {
14-
val toSend = "ping"
15-
val toReceive = "pong"
16-
val env = new ExtraEnv(secondaryOutputDir = "tmp")
6+
testSuccess(WindowsPluginFrontend)
7+
}
178

18-
val fakeGenerator = new ProtocCodeGenerator {
19-
override def run(request: Array[Byte]): Array[Byte] = {
20-
request mustBe (toSend.getBytes ++ env.toByteArrayAsField)
21-
toReceive.getBytes
22-
}
23-
}
24-
val (path, state) = WindowsPluginFrontend.prepare(
25-
fakeGenerator,
26-
env
27-
)
28-
val actualOutput = scala.collection.mutable.Buffer.empty[String]
29-
val process = sys.process
30-
.Process(path.toAbsolutePath.toString)
31-
.#<(new ByteArrayInputStream(toSend.getBytes))
32-
.run(ProcessLogger(o => actualOutput.append(o)))
33-
process.exitValue()
34-
actualOutput.mkString mustBe toReceive
35-
WindowsPluginFrontend.cleanup(state)
9+
it must "not hang if there is an error in generator" in {
10+
testFailure(WindowsPluginFrontend)
3611
}
3712
}
3813
}

0 commit comments

Comments
 (0)