Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch PluginFrontend to domain sockets on macOS #380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package protocbridge.frontend

import org.newsclub.net.unix.AFUNIXServerSocket
import protocbridge.{ExtraEnv, ProtocCodeGenerator}

import java.net.ServerSocket
import java.nio.file.attribute.PosixFilePermission
import java.nio.file.{Files, Path}
import java.{util => ju}
Expand All @@ -8,20 +12,52 @@ import java.{util => ju}
*
* Creates a server socket and uses `nc` to communicate with the socket. We use
* a server socket instead of named pipes because 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.
* macOS: https://github.com/scalapb/protoc-bridge/issues/366
*
* Since `nc` is widely available on macOS, this is the simplest alternative
* for macOS. However, raw `nc` is also not very reliable on macOS:
* https://github.com/scalapb/protoc-bridge/issues/379
*
* The most reliable way to communicate is found to be with a domain socket and
* a server-side read timeout, which are implemented here.
*/
object MacPluginFrontend extends SocketBasedPluginFrontend {
case class InternalState(
shellScript: Path,
tempDirPath: Path,
socketPath: Path,
serverSocket: ServerSocket
)

override def prepare(
plugin: ProtocCodeGenerator,
env: ExtraEnv
): (Path, InternalState) = {
val tempDirPath = Files.createTempDirectory("protocbridge")
val socketPath = tempDirPath.resolve("socket")
val serverSocket = AFUNIXServerSocket.bindOn(socketPath, true)
val sh = createShellScript(socketPath)

runWithSocket(plugin, env, serverSocket)

(sh, InternalState(sh, tempDirPath, socketPath, serverSocket))
}

override def cleanup(state: InternalState): Unit = {
state.serverSocket.close()
if (sys.props.get("protocbridge.debug") != Some("1")) {
Files.delete(state.tempDirPath)
Files.delete(state.shellScript)
}
}

protected def createShellScript(port: Int): Path = {
private def createShellScript(socketPath: Path): Path = {
val shell = sys.env.getOrElse("PROTOCBRIDGE_SHELL", "/bin/sh")
// We use 127.0.0.1 instead of localhost for the (very unlikely) case that localhost is missing from /etc/hosts.
val scriptName = PluginFrontend.createTempFile(
"",
s"""|#!$shell
|set -e
|nc 127.0.0.1 $port
|nc -U "$socketPath"
""".stripMargin
)
val perms = new ju.HashSet[PosixFilePermission]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,30 @@ 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(
protected def runWithSocket(
plugin: ProtocCodeGenerator,
env: ExtraEnv
): (Path, InternalState) = {
val ss = new ServerSocket(0) // Bind to any available port.
val sh = createShellScript(ss.getLocalPort)

env: ExtraEnv,
serverSocket: ServerSocket
): Unit = {
Future {
blocking {
// Accept a single client connection from the shell script.
val client = ss.accept()
val client = serverSocket.accept()
// It's found on macOS that a `junixsocket` domain socket server
// might not receive the EOF sent by the other end, leading to a hang:
// https://github.com/scalapb/protoc-bridge/issues/379
// However, confusingly, adding an arbitrary read timeout resolves the issue.
// We thus add a read timeout of 1 minute here, which should be more than enough.
// It also helps to prevent an infinite hang on both Windows and macOS due to
// unexpected issues.
client.setSoTimeout(60000)
try {
val response =
PluginFrontend.runWithInputStream(
Expand All @@ -36,16 +40,5 @@ abstract class SocketBasedPluginFrontend extends PluginFrontend {
}
}
}

(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
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,38 @@
package protocbridge.frontend

import java.nio.file.{Path, Paths}
import protocbridge.{ExtraEnv, ProtocCodeGenerator}

import java.net.ServerSocket
import java.nio.file.{Files, 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 SocketBasedPluginFrontend {
case class InternalState(shellScript: Path, serverSocket: ServerSocket)

override def prepare(
plugin: ProtocCodeGenerator,
env: ExtraEnv
): (Path, InternalState) = {
val ss = new ServerSocket(0) // Bind to any available port.
val sh = createShellScript(ss.getLocalPort)

runWithSocket(plugin, env, ss)

(sh, InternalState(sh, ss))
}

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 = {
private def createShellScript(port: Int): Path = {
val classPath =
Paths.get(getClass.getProtectionDomain.getCodeSource.getLocation.toURI)
val classPathBatchString = classPath.toString.replace("%", "%%")
Expand Down
3 changes: 2 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ lazy val bridge: Project = project
"org.scalatest" %% "scalatest" % "3.2.19" % "test",
"org.scalacheck" %% "scalacheck" % "1.18.1" % "test",
"org.scala-lang.modules" %% "scala-collection-compat" % "2.12.0" % "test",
"io.get-coursier" %% "coursier" % coursierVersion % "test"
"io.get-coursier" %% "coursier" % coursierVersion % "test",
"com.kohlschutter.junixsocket" % "junixsocket-core" % "2.10.0"
),
scalacOptions ++= (if (scalaVersion.value.startsWith("2.13."))
Seq("-Wconf:origin=.*JavaConverters.*:s")
Expand Down
Loading