-
-
Notifications
You must be signed in to change notification settings - Fork 286
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
protoc-bridge
builds hang when scalapb.ScalaPbCodeGenerator
doesn't catch Throwable
#1771
Comments
Quick update: I just noticed the latest protoc-bridge implementation of So feel free to close this if a new |
This version contains this update to `protocbridge.frontend.PluginFrontend.runWithInputStream()`: - https://github.com/scalapb/protoc-bridge/blob/c574d50eaee5b800fd54493fe25c4e0eed3b9def/bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala#L107-L122 This change originally came from: - https://github.com/scalapb/protoc-bridge/blob/d0d56f635d13f7efaa2755ed0d2d66bdef18b588/bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala#L107-L122 - scalapb/protoc-bridge#367 This should close scalapb/ScalaPB#1771 and obviate the need for the `try`/`catch` blocks added in bazelbuild#1630 and bazelbuild#1637, and the `ScalaPBCodeGenerator` implementation added in bazelbuild#1648. The update requires special handling in `scripts/create_repository.py` to ensure that only Scala 3.3 and up receive the `protoc-bridge_3` version. Scala 3.1 and 3.2 still receive the `protoc-bridge_2.13` version. However, the `protoc-bridge_3` version currently fails with the following (setting `RULES_SCALA_TEST_ONLY="test_scala_version 3.3.4"` fails the same way): ```txt $ RULES_SCALA_TEST_ONLY="test_scala_version 3.6.2" \ ./test_thirdparty_version.sh ERROR: third_party/test/proto/BUILD.bazel:4:14: ProtoScalaPBRule third_party/test/proto/proto_jvm_extra_protobuf_generator_scalapb.srcjar failed: (Exit 1): scalapb_worker failed: error executing command (from target //third_party/test/proto:proto) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) --scala_out: java.lang.NoClassDefFoundError: Could not initialize class scalapb.ScalaPbCodeGenerator$ at scripts.ScalaPbCodeGenerator$.process(ScalaPbCodeGeneratorWrapper.scala:8) at protocgen.CodeGenApp.run(CodeGenApp.scala:48) at protocgen.CodeGenApp.run$(CodeGenApp.scala:41) at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper.scala:5) at protocgen.CodeGenApp.run(CodeGenApp.scala:33) at protocgen.CodeGenApp.run$(CodeGenApp.scala:32) at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper.scala:5) at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:48) at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:113) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1$$anonfun$1(SocketBasedPluginFrontend.scala:31) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1$$anonfun$adapted$1(SocketBasedPluginFrontend.scala:37) at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1$$anon$2.block(ExecutionContextImpl.scala:60) at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3118) at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1.blockOn(ExecutionContextImpl.scala:71) at scala.concurrent.package$.blocking(package.scala:124) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1(SocketBasedPluginFrontend.scala:37) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$adapted$1(SocketBasedPluginFrontend.scala:38) at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:687) at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:467) at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183) java.lang.RuntimeException: Exit with code 1 at scala.sys.package$.error(package.scala:27) at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44) at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96) at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49) at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39) at scripts.ScalaPBWorker.main(ScalaPBWorker.scala) ERROR: third_party/test/proto/BUILD.bazel:4:14 Building source jar third_party/test/proto/proto_scalapb-src.jar failed: (Exit 1): scalapb_worker failed: error executing command (from target //third_party/test/proto:proto) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) ```
I've confirmed in my |
This required updates to `scripts/create_repository.py` to accommodate comparisons between `_2.13` and `_3` versions of ScalaPB artifacts. The `_3` versions of the most recent ScalaPB artifacts are only available for Scala 3.3 and later, and `compilerplugin_3` currently still depends on `protoc-gen_2.13`. The `MavenCoordinates` dataclass in `scripts/create_repository.py` has new `unversioned_artifact`, `scala_version`, and `artifact_name` members. Updates to `MavenCoordinates.is_newer_than()` now compare objects with matching `artifact_name` values, and take the `scala_version` into account. These changes then precipitated: - Replacing the `artifact_name()` call with `artifact_name` accesses. - Replacing `ArtifactLabelMaker._remove_scala_version_suffix` with `MavenCoordinates.unversioned_artifact`. - Extracting `__compare_versions` from `is_newer_than` to reuse the same code for comparing Scala versions and artifact versions. (Properly reversed the use of `lhs` and `rhs` in the process.) This change also removes an unnecessary `try`/`catch` block from `scripts.ScalaPbCodeGenerator.process()` for Scala 2.12 and above. The block is still required for the Scala 2.11 implementation of `scripts.ScalaPbCodeGenerator.run()` and in `scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator.run()`. Finally, this change adds notes in a few places to indicate where support remains for Scala 2.11. This supporting code can ultimately be removed if we ever decide to drop Scala 2.11 support. --- Tested by setting the `protobuf` dependencies to a versions I know will break `ScalaPB` 0.11.17: - `abseil-cpp`: 20220623.1 => 20240722.0 - `protobuf`: v21.7 => v26.1 I temporarily removed the `try`/`catch` block for `Throwable` from `scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator.run`. I then ran `bazel shutdown` to stop persistent `ProtoScalaPBRule` workers. After that, building with Bazel 7.4.1 (since Bazel 6.5.0 can't build `abseil-cpp` 20240722.0 without setting C++14 compiler flags) crashed as expected, instead of hanging. Undoing the `abseil-cpp` and `protobuf` changes and building then produced a working build. ```txt $ USE_BAZEL_VERSION=7.4.1 bazel build //third_party/test/proto:scala [ ...snip... ] ERROR: third_party/test/proto/BUILD.bazel:4:14: ProtoScalaPBRule third_party/test/proto/proto_jvm_extra_protobuf_generator_scalapb.srcjar failed: (Exit 1): scalapb_worker failed: error executing ProtoScalaPBRule command (from target //third_party/test/proto:proto) bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) --jvm_extra_protobuf_generator_out: java.lang.VerifyError: Cannot inherit from final class at java.base/java.lang.ClassLoader.defineClass1(Native Method) at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1022) at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174) at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:800) at java.base/jdk.internal.loader.BuiltinClassLoader$4.run(BuiltinClassLoader.java:711) at java.base/jdk.internal.loader.BuiltinClassLoader$4.run(BuiltinClassLoader.java:706) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:719) at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:621) at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:579) at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:527) at scalapb.options.Scalapb.<clinit>(Scalapb.java:24835) at scalapb.options.compiler.Scalapb$.registerAllExtensions(Scalapb.scala:8) at scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator$.run(ExtraProtobufGenerator.scala:52) at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:48) at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:113) at protocbridge.frontend.SocketBasedPluginFrontend.$anonfun$prepare$2(SocketBasedPluginFrontend.scala:31) at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23) at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1$$anon$2.block(ExecutionContextImpl.scala:75) at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3118) at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1.blockOn(ExecutionContextImpl.scala:87) at scala.concurrent.package$.blocking(package.scala:146) at protocbridge.frontend.SocketBasedPluginFrontend.$anonfun$prepare$1(SocketBasedPluginFrontend.scala:23) at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23) at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:659) at scala.util.Success.$anonfun$map$1(Try.scala:255) at scala.util.Success.map(Try.scala:213) at scala.concurrent.Future.$anonfun$map$1(Future.scala:292) at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:42) at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:74) at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183) java.lang.RuntimeException: Exit with code 1 at scala.sys.package$.error(package.scala:30) at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44) at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96) at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49) at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39) at scripts.ScalaPBWorker.main(ScalaPBWorker.scala) Target //third_party/test/proto:scala failed to build ERROR: third_party/test/proto/BUILD.bazel:4:14 scala @//third_party/test/proto:proto failed: (Exit 1): scalapb_worker failed: error executing ProtoScalaPBRule command (from target //third_party/test/proto:proto) bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) ``` --- The initial motivation for this change was to try to eliminate the need for the custom `scripts.ScalaPbCodeGenerator` implementation by upgrading to `protoc-bridge` 0.9.8. This version contains a change to `catch` any `Throwable` objects raised by a generator implementation in `protocbridge.frontend.PluginFrontend.runWithInputStream()`. - https://github.com/scalapb/protoc-bridge/blob/c574d50eaee5b800fd54493fe25c4e0eed3b9def/bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala#L107-L122 This change originally came from: - https://github.com/scalapb/protoc-bridge/blob/d0d56f635d13f7efaa2755ed0d2d66bdef18b588/bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala#L107-L122 - scalapb/protoc-bridge#367 I closed scalapb/ScalaPB#1771 as a result, and reexamined the `try`/`catch` blocks added in bazelbuild#1630 and bazelbuild#1637, and the `scripts.ScalaPBCodeGenerator` implementations added in bazelbuild#1648. However, the last `protoc-bridge` version to support Scala 2.11 is 0.7.14. Scala 2.11 won't be able to use `protoc-bridge` 0.9.8, so we still need to `catch` any `Throwable`s ourselves. This includes the `catch` blocks from the Scala 2.11 `scripts.ScalaPbCodeGenerator` and `scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator.run`. --- In `protoc-bridge` 0.9.7 and 0.9.8, `protocbridge.ProtocCodeGenerator` declares a `run()` method with no implementation (and no `try`/`catch` block). - https://github.com/scalapb/protoc-bridge/blob/v0.9.7/bridge/src/main/scala/protocbridge/ProtocCodeGenerator.scala#L5 In `protoc-gen` 0.9.7 and 0.9.8, `protogen.CodeGenApp` implements `protocbridge.ProtocCodeGenerator`, and `CodeGenApp.run()` wraps a call to `CodeGenApp.process()` inside a `try`/`catch` block. - https://github.com/scalapb/protoc-bridge/blob/v0.9.7/protoc-gen/src/main/scala/protocgen/CodeGenApp.scala#L41-L56 `scalapb.ScalaPbCodeGenerator` from `compilerplugin` 0.11.17 extends `CodeGenApp` and implements `process()`. For this version of `ScalaPbCodeGenerator`, available in Scala 2.12 and later, there's no need for a `try`/`catch` wrapper on our end. - https://github.com/scalapb/ScalaPB/blob/v0.11.17/compiler-plugin/src/main/scala/scalapb/ScalaPbCodeGenerator.scala#L11 However, the last available `compilerplugin` version for Scala 2.11 is 0.9.8. `scalapb.ScalaPbCodeGenerator` from that version implements `protocbridge.ProtocCodeGenerator` and has a `try`/`catch` in its `run()` method. However, the `Scalapb.registerAllExtensions(registry)` call on line 14 lies outside this block, producing the crash described in bazelbuild#1648 (commit 23ae356). This is why we need the Scala 2.11 implementation of `scripts.ScalaPbCodeGenerator`. - https://github.com/scalapb/ScalaPB/blob/v0.9.8/compiler-plugin/src/main/scala/scalapb/ScalaPbCodeGenerator.scala#L14 `scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator`, also extends `ProtocCodeGenerator` directly, instead of implementing `CodeGenApp` (so it also builds with Scala 2.11). This is why `ExtraProtobufGenerator.run()` hangs when we remove the `try`/`catch` block and run with `protoc-bridge` 0.9.7 and `protobuf` > v25.5, even with later Scala versions. --- Since `scalapb.ScalaPbCodeGenerator` from `compilerplugin` 0.11.17 implements `CodeGenApp`, `scripts.ScalaPbCodeGenerator` for Scala 2.12 should be unnecessary. The only reason we need it is to maintain the same interface as the Scala 2.11 implementation. --- Before the `MavenCoordinates` changes, `scripts/create_repository.py` would ScalaPB flip ScalaPB artifacts between their `_2.13` and `_3` versions on subsequent runs (e.g., `protoc-bridge_2.13` vs. `protoc-bridge_3`). See the comments added within `MavenCoordinates.new()` for details. --- Building Scala 3.3 and later versions with `compilerplugin_3` and `protoc-bridge_3` artifacts (instead of `protoc-bridge_2.13` artifacts) produces the following build failure: ```txt $ RULES_SCALA_TEST_ONLY="test_scala_version 3.6.2" \ ./test_thirdparty_version.sh [ ...snip... ] ERROR: third_party/test/proto/BUILD.bazel:4:14: ProtoScalaPBRule third_party/test/proto/proto_jvm_extra_protobuf_generator_scalapb.srcjar failed: (Exit 1): scalapb_worker failed: error executing command (from target //third_party/test/proto:proto) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) --scala_out: java.lang.NoClassDefFoundError: Could not initialize class scalapb.ScalaPbCodeGenerator$ at scripts.ScalaPbCodeGenerator$.process(ScalaPbCodeGeneratorWrapper.scala:8) at protocgen.CodeGenApp.run(CodeGenApp.scala:48) at protocgen.CodeGenApp.run$(CodeGenApp.scala:41) at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper.scala:5) at protocgen.CodeGenApp.run(CodeGenApp.scala:33) at protocgen.CodeGenApp.run$(CodeGenApp.scala:32) at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper.scala:5) at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:48) at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:113) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1$$anonfun$1(SocketBasedPluginFrontend.scala:31) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1$$anonfun$adapted$1(SocketBasedPluginFrontend.scala:37) at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1$$anon$2.block(ExecutionContextImpl.scala:60) at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3118) at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1.blockOn(ExecutionContextImpl.scala:71) at scala.concurrent.package$.blocking(package.scala:124) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1(SocketBasedPluginFrontend.scala:37) at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$adapted$1(SocketBasedPluginFrontend.scala:38) at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:687) at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:467) at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183) java.lang.RuntimeException: Exit with code 1 at scala.sys.package$.error(package.scala:27) at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44) at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96) at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49) at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39) at scripts.ScalaPBWorker.main(ScalaPBWorker.scala) ERROR: third_party/test/proto/BUILD.bazel:4:14 Building source jar third_party/test/proto/proto_scalapb-src.jar failed: (Exit 1): scalapb_worker failed: error executing command (from target //third_party/test/proto:proto) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) ```
Neither
scalapb.ScalaPbCodeGenerator.run()
(ScalaPB <= 0.9.8) norscalapb.ScalaPbCodeGenerator.process()
(ScalaPB >= 0.11.17) catchThrowable
and convert it to an error response. When an exception occurs, theFuture
running either of these methods fails to resolve. It never writes a response to theprotoc-bridge
pipe, causing builds to hang.I've opened bazelbuild/rules_scala#1648, which contains bazelbuild/rules_scala@de8214b. That commit contains wrappers for both
scalapb.ScalaPbCodeGenerator
implementations that catchThrowable
and allow the build to fail with an error.If it's desirable, I'm happy to contribute these changes, or a better implementation if suggested, to ScalaPB itself (for any or all of the 0.9.x, 0.11.x, and 1.0.x branches).
Background: I've been Bzlmodifying
rules_scala
per bazelbuild/rules_scala#1482, and as a side quest, I've diagnosed hanging build issues related to ScalaPB in bazelbuild/rules_scala#1647. That issue contains extensive details, and has a table at the top showing which versions of ScalaPB are compatible with which versions of Scala and the protobuf Bazel module.The text was updated successfully, but these errors were encountered: