From 716210fe84d452e3db81e8837679a79e29b0f4e5 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Mon, 4 Nov 2024 06:57:15 -0500 Subject: [PATCH] Move Throwable catch to ExtraProtobufGenerator.run (#1637) This catches errors that happen at any point in `ExtraProtobufGenerator.run`, not just within `handleCodeGeneratorRequest`. This, in turn, prevents more cases of `ProtoScalaPBRule` aspect workers hanging. Part of #1482. As I started experimenting with updating gRPC and ScalaPB libs, I bumped the ScalaPB libs to 1.0.0-alpha.1 to try to resolve a problem. The `ProtoScalaPBRule` workers then started hanging again. After pulling the `catch ... Throwable` block from #1630 up into `ExtraProtobufGenerator.run`, I got the following stack trace: ```txt $ bazel build //test/proto/... ERROR: .../external/com_google_protobuf/BUILD.bazel:334:14: ProtoScalaPBRule external/com_google_protobuf/wrappers_proto_jvm_extra_protobuf_generator_scalapb.srcjar failed: (Exit 1): scalapb_worker failed: error executing command (from target @com_google_protobuf//:wrappers_proto) bazel-out/.../bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) --jvm_extra_protobuf_generator_out: java.lang.IllegalAccessError: class scalapb.options.Scalapb$ScalaPbOptions tried to access method 'com.google.protobuf.LazyStringArrayList com.google.protobuf.LazyStringArrayList.emptyList()' (scalapb.options.Scalapb$ScalaPbOptions and com.google.protobuf.LazyStringArrayList are in unnamed module of loader 'app') at scalapb.options.Scalapb$ScalaPbOptions.(Scalapb.java:5021) at scalapb.options.Scalapb$ScalaPbOptions.(Scalapb.java:11165) at scalapb.options.Scalapb.(Scalapb.java:24184) at scalapb.options.compiler.Scalapb$.registerAllExtensions(Scalapb.scala:8) at scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator$.run(ExtraProtobufGenerator.scala:48) at protocbridge.frontend.PluginFrontend$.$anonfun$runWithBytes$1(PluginFrontend.scala:51) at scala.util.Try$.apply(Try.scala:213) at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:51) at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:121) at protocbridge.frontend.PosixPluginFrontend$.$anonfun$prepare$2(PosixPluginFrontend.scala:40) 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.PosixPluginFrontend$.$anonfun$prepare$1(PosixPluginFrontend.scala:38) 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) ERROR: .../external/com_google_protobuf/BUILD.bazel:334:14 scala @com_google_protobuf//:wrappers_proto failed: (Exit 1): scalapb_worker failed: error executing command (from target @com_google_protobuf//:wrappers_proto) bazel-out/.../bin/src/scala/scripts/scalapb_worker ... (remaining 2 arguments skipped) ``` Bumping to protobuf v28.2 resolved the issue, at the expense of sacrificing Bazel 6 support, as v21.7 was the last to support Bazel 6. But this issue is orthogonal to the original gRPC test failure issue, which I'll address in a future change. --- .../ExtraProtobufGenerator.scala | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala b/test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala index 4cd4f932e..bc83e37af 100644 --- a/test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala +++ b/test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala @@ -41,14 +41,30 @@ class CustomProtobufGenerator( object ExtraProtobufGenerator extends ProtocCodeGenerator { override def run(req: Array[Byte]): Array[Byte] = { - val registry = ExtensionRegistry.newInstance() - Scalapb.registerAllExtensions(registry) - val request = CodeGeneratorRequest.parseFrom(req) - handleCodeGeneratorRequest(request).toByteArray + val b = CodeGeneratorResponse.newBuilder + + try { + val registry = ExtensionRegistry.newInstance() + Scalapb.registerAllExtensions(registry) + val request = CodeGeneratorRequest.parseFrom(req) + handleCodeGeneratorRequest(request, b) + + } catch { + case e: Throwable => + // Yes, we want to catch _all_ errors and send them back to the + // requestor. Otherwise uncaught errors will cause the generator to + // die and the worker invoking it to hang. + val stackStream = new java.io.ByteArrayOutputStream + e.printStackTrace(new java.io.PrintStream(stackStream)) + b.setError(stackStream.toString()) + } + b.build.toByteArray } - def handleCodeGeneratorRequest(request: CodeGeneratorRequest): CodeGeneratorResponse = { - val b = CodeGeneratorResponse.newBuilder + def handleCodeGeneratorRequest( + request: CodeGeneratorRequest, + b: CodeGeneratorResponse.Builder + ) = { ProtobufGenerator.parseParameters(request.getParameter) match { case Right(params) => try { @@ -73,18 +89,9 @@ object ExtraProtobufGenerator extends ProtocCodeGenerator { } catch { case e: GeneratorException => b.setError(e.message) - case e: Throwable => - // Yes, we want to catch _all_ errors and send them back to the - // requestor. Otherwise uncaught errors will cause the generator to - // die and the worker invoking it to hang. - val stackStream = new java.io.ByteArrayOutputStream - e.printStackTrace(new java.io.PrintStream(stackStream)) - b.setError(stackStream.toString()) } case Left(error) => b.setError(error) } - b.build } - }