Skip to content

Commit 04024dc

Browse files
authored
Fix persistent worker protocol bugs in scalafileextract (#150)
* Fix persistent worker protocol bugs in scalafileextract Fixes three critical bugs that violated the Bazel persistent worker protocol: 1. **Stdout pollution**: The Node.js scalameta parser subprocess inherited os.Stdout, allowing any subprocess output to corrupt the persistent worker protocol stream. Per Bazel's spec, only WorkResponses should be written to stdout. Now redirects subprocess stdout to stderr. 2. **Missing stdout flush**: After writing WorkResponse messages, stdout was not explicitly flushed. Since Go's os.Stdout is buffered, responses could sit in the buffer causing Bazel to hang waiting for the worker. Now calls os.Stdout.Sync() after each response. 3. **Error handling**: When batchWork() failed, the worker terminated instead of reporting the error via WorkResponse. This crashed the entire worker process on any parse error, requiring Bazel to restart the worker and losing persistent worker performance benefits. Now reports errors via WorkResponse.ExitCode and .Output fields. * Use self-hosted runner * Use remotejdk * use buffered writer for stdout
1 parent b796d8b commit 04024dc

5 files changed

Lines changed: 22 additions & 23 deletions

File tree

.bazelrc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# use remove java
2+
build --java_runtime_version=remotejdk_21
3+
14
# Keep until @io_bazel_rules_scala is upgraded
25
build --incompatible_java_common_parameters=false
36

.github/workflows/ci.yaml

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,32 +14,15 @@ on:
1414
jobs:
1515
test:
1616
# The type of runner that the job will run on
17-
runs-on: ubuntu-latest
17+
runs-on: self-hosted
1818

1919
# Steps represent a sequence of tasks that will be executed as part of the job
2020
steps:
2121
- uses: actions/checkout@v3
2222

2323
- uses: bazelbuild/setup-bazelisk@v3
2424

25-
- name: Mount bazel action cache
26-
uses: actions/cache@v4
27-
if: always()
28-
with:
29-
path: "~/.cache/bazel"
30-
key: bazel
31-
32-
- name: Mount bazel repo cache
33-
uses: actions/cache@v4
34-
if: always()
35-
with:
36-
path: "~/.cache/bazel-repo"
37-
key: bazel-repo
38-
3925
- name: bazel test
40-
env:
41-
# Bazelisk will download bazel to here, ensure it is cached between runs.
42-
XDG_CACHE_HOME: ~/.cache/bazel-repo
4326
run: >-
4427
bazel
4528
--bazelrc=.github/workflows/ci.bazelrc

cmd/scalafileextract/scalafileextract.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"bufio"
45
"context"
56
"flag"
67
"fmt"
@@ -84,6 +85,9 @@ func run(args []string) error {
8485
}
8586

8687
func persistentWork(cfg *Config) error {
88+
// Use a buffered writer for stdout to ensure we can flush properly
89+
stdout := bufio.NewWriter(os.Stdout)
90+
8791
for {
8892
var req wppb.WorkRequest
8993
if err := protobuf.ReadDelimitedFrom(&req, os.Stdin); err != nil {
@@ -106,12 +110,18 @@ func persistentWork(cfg *Config) error {
106110
batchCfg.Cwd = cfg.Cwd
107111

108112
if err := batchWork(&batchCfg); err != nil {
109-
return fmt.Errorf("performing persistent batch!: %v", err)
113+
// Don't terminate the worker on batch errors; report via WorkResponse
114+
resp.ExitCode = 1
115+
resp.Output = fmt.Sprintf("performing persistent batch: %v", err)
110116
}
111117

112-
if err := protobuf.WriteDelimitedTo(&resp, os.Stdout); err != nil {
118+
if err := protobuf.WriteDelimitedTo(&resp, stdout); err != nil {
113119
return fmt.Errorf("writing work response: %v", err)
114120
}
121+
// Flush stdout to ensure the WorkResponse is immediately available to Bazel
122+
if err := stdout.Flush(); err != nil {
123+
return fmt.Errorf("flushing work response: %v", err)
124+
}
115125
}
116126
return nil
117127
}

pkg/parser/scalameta_parser.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,11 @@ func (s *ScalametaParser) Start() error {
169169
"NODE_PATH=" + processDir,
170170
fmt.Sprintf("PORT=%d", s.httpPort),
171171
}
172-
cmd.Stdin = os.Stdin
173-
cmd.Stdout = os.Stdout
172+
// Don't inherit stdin from parent process
173+
cmd.Stdin = nil
174+
// Redirect stdout to stderr to avoid polluting the persistent worker protocol
175+
// (only WorkResponses should go to stdout in persistent worker mode)
176+
cmd.Stdout = os.Stderr
174177
cmd.Stderr = os.Stderr
175178
s.cmd = cmd
176179

pkg/provider/semanticdb_provider.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func (cr *SemanticdbProvider) CanProvide(dep *resolver.ImportLabel, expr build.E
136136
func (r *SemanticdbProvider) ParseScalaRule(kind string, from label.Label, dir string, srcs ...string) (*sppb.Rule, error) {
137137
rule, err := r.delegate.ParseScalaRule(kind, from, dir, srcs...)
138138
if err != nil {
139-
return nil, err
139+
return nil, fmt.Errorf("semanticdb: %v", err)
140140
}
141141
for _, file := range rule.Files {
142142
r.visitFile(from.Pkg, file)

0 commit comments

Comments
 (0)