Switch PosixPluginFrontend to sockets

Author: bell-db

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

@@ -0,0 +1,33 @@
+package protocbridge.frontend
+
+import java.nio.file.attribute.PosixFilePermission
+import java.nio.file.{Files, Path}
+import java.{util => ju}
+
+/** PluginFrontend for macOS.
+  *
+  * Creates a server socket and uses `nc` to communicate with the socket.
+  * Named pipes are unreliable on macOS: https://github.com/scalapb/protoc-bridge/issues/366.
+  * Since `nc` is widely available on macOS, this is the simplest and most reliable solution for macOS.
+  */
+object MacPluginFrontend extends SocketBasedPluginFrontend {
+
+  protected def createShellScript(port: Int): Path = {
+    val shell = sys.env.getOrElse("PROTOCBRIDGE_SHELL", "/bin/sh")
+    val scriptName = PluginFrontend.createTempFile(
+      "",
+      s"""|#!$shell
+          |set -e
+          |nc localhost $port
+      """.stripMargin
+    )
+    val perms = new ju.HashSet[PosixFilePermission]
+    perms.add(PosixFilePermission.OWNER_EXECUTE)
+    perms.add(PosixFilePermission.OWNER_READ)
+    Files.setPosixFilePermissions(
+      scriptName,
+      perms
+    )
+    scriptName
+  }
+}

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

@@ -2,8 +2,7 @@ package protocbridge.frontend
 
 import java.io.{ByteArrayOutputStream, InputStream, PrintWriter, StringWriter}
 import java.nio.file.{Files, Path}
-
-import protocbridge.{ProtocCodeGenerator, ExtraEnv}
+import protocbridge.{ExtraEnv, ProtocCodeGenerator}
 
 import scala.util.Try
 
@@ -133,8 +132,11 @@ object PluginFrontend {
 
   def isWindows: Boolean = sys.props("os.name").startsWith("Windows")
 
+  def isMac: Boolean = sys.props("os.name").startsWith("Mac") || sys.props("os.name").startsWith("Darwin")
+
   def newInstance: PluginFrontend = {
     if (isWindows) WindowsPluginFrontend
+    else if (isMac) MacPluginFrontend
     else PosixPluginFrontend
   }
 }

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

@@ -12,10 +12,12 @@ import scala.concurrent.ExecutionContext.Implicits.global
 import scala.sys.process._
 import java.{util => ju}
 
-/** PluginFrontend for Unix-like systems (Linux, Mac, etc)
+/** PluginFrontend for Unix-like systems <b>except macOS</b> (Linux, FreeBSD, etc)
   *
   * Creates a pair of named pipes for input/output and a shell script that
   * communicates with them.
+  * Compared with `SocketBasedPluginFrontend`, this frontend should be more performant
+  * and doesn't rely on `nc` that might not be available in some distributions.
   */
 object PosixPluginFrontend extends PluginFrontend {
   case class InternalState(

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

@@ -0,0 +1,61 @@
+package protocbridge.frontend
+
+import protocbridge.{ExtraEnv, ProtocCodeGenerator}
+
+import java.net.ServerSocket
+import java.nio.file.{Files, Path}
+import scala.concurrent.ExecutionContext.Implicits.global
+import scala.concurrent.{Future, blocking}
+
+/** PluginFrontend for Windows and macOS where a server socket is used.
+  */
+abstract class SocketBasedPluginFrontend extends PluginFrontend {
+  case class InternalState(serverSocket: ServerSocket, shellScript: Path)
+
+  override def prepare(
+      plugin: ProtocCodeGenerator,
+      env: ExtraEnv
+  ): (Path, InternalState) = {
+    val ss = new ServerSocket(0) // Bind to any available port.
+    val sh = createShellScript(ss.getLocalPort)
+
+    Future {
+      blocking {
+        // Accept a single client connection from the shell script.
+        val client = ss.accept()
+
+        try {
+          val cis = client.getInputStream
+          val response = PluginFrontend.runWithInputStream(plugin, cis, env)
+
+          val cos = client.getOutputStream
+          cos.write(response)
+        } catch {
+          case e: Throwable =>
+            // Handles rare exceptions not already gracefully handled in `runWithBytes`.
+            // Such exceptions aren't converted to `CodeGeneratorResponse`
+            // because `cis` might not be fully consumed,
+            // therefore the plugin shell script might hang on `nc` write,
+            // and never get to `nc` read and consume `CodeGeneratorResponse`.
+            //
+            // Instead, we simply force close the client connection,
+            // so that the plugin shell script can exit.
+            System.err.println("Exception occurred in PluginFrontend outside runWithBytes")
+            e.printStackTrace(System.err)
+        } finally {
+           client.close()
+        }
+      }
+    }
+    (sh, InternalState(ss, sh))
+  }
+
+  override def cleanup(state: InternalState): Unit = {
+    state.serverSocket.close()
+    if (sys.props.get("protocbridge.debug") != Some("1")) {
+      Files.delete(state.shellScript)
+    }
+  }
+
+  protected def createShellScript(port: Int): Path
+}

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

@@ -1,53 +1,15 @@
 package protocbridge.frontend
 
-import java.net.ServerSocket
-import java.nio.file.{Files, Path, Paths}
-
-import protocbridge.ExtraEnv
-import protocbridge.ProtocCodeGenerator
-
-import scala.concurrent.blocking
-
-import scala.concurrent.ExecutionContext.Implicits.global
-import scala.concurrent.Future
+import java.nio.file.{Path, Paths}
 
 /** A PluginFrontend that binds a server socket to a local interface. The plugin
   * is a batch script that invokes BridgeApp.main() method, in a new JVM with
   * the same parameters as the currently running JVM. The plugin will
   * communicate its stdin and stdout to this socket.
   */
-object WindowsPluginFrontend extends PluginFrontend {
-
-  case class InternalState(batFile: Path)
-
-  override def prepare(
-      plugin: ProtocCodeGenerator,
-      env: ExtraEnv
-  ): (Path, InternalState) = {
-    val ss = new ServerSocket(0)
-    val state = createWindowsScript(ss.getLocalPort)
-
-    Future {
-      blocking {
-        val client = ss.accept()
-        val response =
-          PluginFrontend.runWithInputStream(plugin, client.getInputStream, env)
-        client.getOutputStream.write(response)
-        client.close()
-        ss.close()
-      }
-    }
-
-    (state.batFile, state)
-  }
-
-  override def cleanup(state: InternalState): Unit = {
-    if (sys.props.get("protocbridge.debug") != Some("1")) {
-      Files.delete(state.batFile)
-    }
-  }
+object WindowsPluginFrontend extends SocketBasedPluginFrontend {
 
-  private def createWindowsScript(port: Int): InternalState = {
+  protected def createShellScript(port: Int): Path = {
     val classPath =
       Paths.get(getClass.getProtectionDomain.getCodeSource.getLocation.toURI)
     val classPathBatchString = classPath.toString.replace("%", "%%")
@@ -62,6 +24,6 @@ object WindowsPluginFrontend extends PluginFrontend {
         ].getName} $port
         """.stripMargin
     )
-    InternalState(batchFile)
+    batchFile
   }
 }

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

@@ -0,0 +1,10 @@
+package protocbridge.frontend
+
+class MacPluginFrontendSpec extends OsSpecificFrontendSpec {
+  if (PluginFrontend.isMac) {
+    it must "execute a program that forwards input and output to given stream" in {
+      val state = testPluginFrontend(MacPluginFrontend)
+      state.serverSocket.isClosed mustBe true
+    }
+  }
+}

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

@@ -0,0 +1,46 @@
+package protocbridge.frontend
+
+import org.apache.commons.io.IOUtils
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.must.Matchers
+import protocbridge.{ExtraEnv, ProtocCodeGenerator}
+
+import java.io.ByteArrayOutputStream
+import scala.sys.process.ProcessIO
+import scala.util.Random
+
+class OsSpecificFrontendSpec extends AnyFlatSpec with Matchers {
+
+  protected def testPluginFrontend(frontend: PluginFrontend): frontend.InternalState = {
+    val random = new Random()
+    val toSend = Array.fill(123)(random.nextInt(256).toByte)
+    val toReceive = Array.fill(456)(random.nextInt(256).toByte)
+    val env = new ExtraEnv(secondaryOutputDir = "tmp")
+
+    val fakeGenerator = new ProtocCodeGenerator {
+      override def run(request: Array[Byte]): Array[Byte] = {
+        request mustBe (toSend ++ env.toByteArrayAsField)
+        toReceive
+      }
+    }
+    val (path, state) = frontend.prepare(
+      fakeGenerator,
+      env
+    )
+    val actualOutput = new ByteArrayOutputStream()
+    val process = sys.process
+      .Process(path.toAbsolutePath.toString)
+      .run(new ProcessIO(writeInput => {
+        writeInput.write(toSend)
+        writeInput.close()
+      }, processOutput => {
+        IOUtils.copy(processOutput, actualOutput)
+        processOutput.close()
+      }, _.close()))
+    process.exitValue()
+    actualOutput.toByteArray mustBe toReceive
+    frontend.cleanup(state)
+
+    state
+  }
+}

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

@@ -1,45 +1,9 @@
 package protocbridge.frontend
 
-import org.apache.commons.io.IOUtils
-import org.scalatest.flatspec.AnyFlatSpec
-import org.scalatest.matchers.must.Matchers
-import protocbridge.{ExtraEnv, ProtocCodeGenerator}
-
-import java.io.ByteArrayOutputStream
-import scala.sys.process.ProcessIO
-import scala.util.Random
-
-class PosixPluginFrontendSpec extends AnyFlatSpec with Matchers {
-  if (!PluginFrontend.isWindows) {
+class PosixPluginFrontendSpec extends OsSpecificFrontendSpec {
+  if (!PluginFrontend.isWindows && !PluginFrontend.isMac) {
     it must "execute a program that forwards input and output to given stream" in {
-      val random = new Random()
-      val toSend = Array.fill(123)(random.nextInt(256).toByte)
-      val toReceive = Array.fill(456)(random.nextInt(256).toByte)
-      val env = new ExtraEnv(secondaryOutputDir = "tmp")
-
-      val fakeGenerator = new ProtocCodeGenerator {
-        override def run(request: Array[Byte]): Array[Byte] = {
-          request mustBe (toSend ++ env.toByteArrayAsField)
-          toReceive
-        }
-      }
-      val (path, state) = PosixPluginFrontend.prepare(
-        fakeGenerator,
-        env
-      )
-      val actualOutput = new ByteArrayOutputStream()
-      val process = sys.process
-        .Process(path.toAbsolutePath.toString)
-        .run(new ProcessIO(writeInput => {
-          writeInput.write(toSend)
-          writeInput.close()
-        }, processOutput => {
-          IOUtils.copy(processOutput, actualOutput)
-          processOutput.close()
-        }, _.close()))
-      process.exitValue()
-      actualOutput.toByteArray mustBe toReceive
-      PosixPluginFrontend.cleanup(state)
+      testPluginFrontend(PosixPluginFrontend)
     }
   }
 }

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

@@ -1,38 +1,10 @@
 package protocbridge.frontend
 
-import java.io.ByteArrayInputStream
-
-import protocbridge.{ProtocCodeGenerator, ExtraEnv}
-
-import scala.sys.process.ProcessLogger
-import org.scalatest.flatspec.AnyFlatSpec
-import org.scalatest.matchers.must.Matchers
-
-class WindowsPluginFrontendSpec extends AnyFlatSpec with Matchers {
+class WindowsPluginFrontendSpec extends OsSpecificFrontendSpec {
   if (PluginFrontend.isWindows) {
     it must "execute a program that forwards input and output to given stream" in {
-      val toSend = "ping"
-      val toReceive = "pong"
-      val env = new ExtraEnv(secondaryOutputDir = "tmp")
-
-      val fakeGenerator = new ProtocCodeGenerator {
-        override def run(request: Array[Byte]): Array[Byte] = {
-          request mustBe (toSend.getBytes ++ env.toByteArrayAsField)
-          toReceive.getBytes
-        }
-      }
-      val (path, state) = WindowsPluginFrontend.prepare(
-        fakeGenerator,
-        env
-      )
-      val actualOutput = scala.collection.mutable.Buffer.empty[String]
-      val process = sys.process
-        .Process(path.toAbsolutePath.toString)
-        .#<(new ByteArrayInputStream(toSend.getBytes))
-        .run(ProcessLogger(o => actualOutput.append(o)))
-      process.exitValue()
-      actualOutput.mkString mustBe toReceive
-      WindowsPluginFrontend.cleanup(state)
+      val state = testPluginFrontend(WindowsPluginFrontend)
+      state.serverSocket.isClosed mustBe true
     }
   }
 }