Skip to content

Commit 1ed4511

Browse files
committed
Switch PosixPluginFrontend to sockets
1 parent 9ba41d9 commit 1ed4511

File tree

2 files changed

+31
-53
lines changed

2 files changed

+31
-53
lines changed

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

+30-53
Original file line numberDiff line numberDiff line change
@@ -1,98 +1,75 @@
11
package protocbridge.frontend
22

3-
import java.nio.file.{Files, Path}
3+
import protocbridge.{ExtraEnv, ProtocCodeGenerator}
44

5-
import protocbridge.ProtocCodeGenerator
6-
import protocbridge.ExtraEnv
5+
import java.net.ServerSocket
76
import java.nio.file.attribute.PosixFilePermission
8-
9-
import scala.concurrent.blocking
10-
import scala.concurrent.Future
11-
import scala.concurrent.ExecutionContext.Implicits.global
12-
import scala.sys.process._
7+
import java.nio.file.{Files, Path}
138
import java.{util => ju}
9+
import scala.concurrent.ExecutionContext.Implicits.global
10+
import scala.concurrent.{Future, blocking}
1411

1512
/** PluginFrontend for Unix-like systems (Linux, Mac, etc)
1613
*
17-
* Creates a pair of named pipes for input/output and a shell script that
18-
* communicates with them.
14+
* Creates a server socket and a shell script that communicates with it.
1915
*/
2016
object PosixPluginFrontend extends PluginFrontend {
21-
case class InternalState(
22-
inputPipe: Path,
23-
outputPipe: Path,
24-
tempDir: Path,
25-
shellScript: Path
26-
)
17+
case class InternalState(serverSocket: ServerSocket, shellScript: Path)
2718

2819
override def prepare(
2920
plugin: ProtocCodeGenerator,
3021
env: ExtraEnv
3122
): (Path, InternalState) = {
32-
val tempDirPath = Files.createTempDirectory("protopipe-")
33-
val inputPipe = createPipe(tempDirPath, "input")
34-
val outputPipe = createPipe(tempDirPath, "output")
35-
val sh = createShellScript(inputPipe, outputPipe)
23+
// We use socket instead of named pipes.
24+
// This is because named pipes are unreliable on macOS: https://github.com/scalapb/protoc-bridge/issues/366.
25+
val ss = new ServerSocket(0) // Bind to any available port.
26+
val sh = createShellScript(ss.getLocalPort)
3627

3728
Future {
3829
blocking {
30+
// Accept a single client connection from the shell script.
31+
val client = ss.accept()
32+
3933
try {
40-
val fsin = Files.newInputStream(inputPipe)
41-
val response = PluginFrontend.runWithInputStream(plugin, fsin, env)
42-
fsin.close()
34+
val cis = client.getInputStream
35+
val response = PluginFrontend.runWithInputStream(plugin, cis, env)
4336

44-
val fsout = Files.newOutputStream(outputPipe)
45-
fsout.write(response)
46-
fsout.close()
37+
val cos = client.getOutputStream
38+
cos.write(response)
4739
} catch {
4840
case e: Throwable =>
4941
// Handles rare exceptions not already gracefully handled in `runWithBytes`.
5042
// Such exceptions aren't converted to `CodeGeneratorResponse`
51-
// because `fsin` might not be fully consumed,
52-
// therefore the plugin shell script might hang on `inputPipe`,
53-
// and never consume `CodeGeneratorResponse`.
43+
// because `cis` might not be fully consumed,
44+
// therefore the plugin shell script might hang on `nc` write,
45+
// and never get to `nc` read and consume `CodeGeneratorResponse`.
46+
//
47+
// Instead, we simply force close the client connection,
48+
// so that the plugin shell script can exit.
5449
System.err.println("Exception occurred in PluginFrontend outside runWithBytes")
5550
e.printStackTrace(System.err)
56-
// Force an exit of the program.
57-
// This is because the plugin shell script might hang on `inputPipe`,
58-
// due to `fsin` not fully consumed.
59-
// Or it might hang on `outputPipe`, due to `fsout` not closed.
60-
// Therefore, the program might be stuck waiting for protoc,
61-
// which in turn is waiting for the plugin shell script.
62-
//
63-
// We can't simply close `fsout` here either,
64-
// because `Files.newOutputStream(outputPipe)` will hang
65-
// if `outputPipe` is not yet opened by the plugin shell script for reading.
66-
sys.exit(1)
51+
} finally {
52+
client.close()
6753
}
6854
}
6955
}
70-
(sh, InternalState(inputPipe, outputPipe, tempDirPath, sh))
56+
(sh, InternalState(ss, sh))
7157
}
7258

7359
override def cleanup(state: InternalState): Unit = {
60+
state.serverSocket.close()
7461
if (sys.props.get("protocbridge.debug") != Some("1")) {
75-
Files.delete(state.inputPipe)
76-
Files.delete(state.outputPipe)
77-
Files.delete(state.tempDir)
7862
Files.delete(state.shellScript)
7963
}
8064
}
8165

82-
private def createPipe(tempDirPath: Path, name: String): Path = {
83-
val pipeName = tempDirPath.resolve(name)
84-
Seq("mkfifo", "-m", "600", pipeName.toAbsolutePath.toString).!!
85-
pipeName
86-
}
87-
88-
private def createShellScript(inputPipe: Path, outputPipe: Path): Path = {
66+
private def createShellScript(port: Int): Path = {
8967
val shell = sys.env.getOrElse("PROTOCBRIDGE_SHELL", "/bin/sh")
9068
val scriptName = PluginFrontend.createTempFile(
9169
"",
9270
s"""|#!$shell
9371
|set -e
94-
|cat /dev/stdin > "$inputPipe"
95-
|cat "$outputPipe"
72+
|nc localhost $port
9673
""".stripMargin
9774
)
9875
val perms = new ju.HashSet[PosixFilePermission]

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

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class PosixPluginFrontendSpec extends AnyFlatSpec with Matchers {
4040
process.exitValue()
4141
actualOutput.toByteArray mustBe toReceive
4242
PosixPluginFrontend.cleanup(state)
43+
state.serverSocket.isClosed mustBe true
4344
}
4445
}
4546
}

0 commit comments

Comments
 (0)