refactor build on save implementation

src/Server.zig

@@ -788,8 +788,8 @@ const Workspace = struct {
         };
         defer args.server.allocator.free(workspace_path);
 
-        workspace.build_on_save = @as(BuildOnSave, undefined);
-        workspace.build_on_save.?.init(.{
+        std.debug.assert(workspace.build_on_save == null);
+        workspace.build_on_save = BuildOnSave.init(.{
             .allocator = args.server.allocator,
             .workspace_path = workspace_path,
             .build_on_save_args = args.server.config.build_on_save_args,
@@ -799,7 +799,6 @@ const Workspace = struct {
             .build_runner_path = build_runner_path,
             .collection = &args.server.diagnostics_collection,
         }) catch |err| {
-            workspace.build_on_save = null;
             log.err("failed to initilize Build-On-Save for '{s}': {}", .{ workspace.uri, err });
             return;
         };

src/features/diagnostics.zig

@@ -431,10 +431,9 @@ fn getErrorBundleFromAstCheck(
     return try error_bundle.toOwnedBundle("");
 }
 
-/// This struct is not relocatable after initilization.
 pub const BuildOnSave = struct {
     allocator: std.mem.Allocator,
-    child_process: std.process.Child,
+    child_process: *std.process.Child,
     thread: std.Thread,
 
     const shared = @import("../build_runner/shared.zig");
@@ -453,8 +452,9 @@ pub const BuildOnSave = struct {
         collection: *DiagnosticsCollection,
     };
 
-    pub fn init(self: *BuildOnSave, options: InitOptions) !void {
-        self.* = undefined;
+    pub fn init(options: InitOptions) !?BuildOnSave {
+        const child_process = try options.allocator.create(std.process.Child);
+        errdefer options.allocator.destroy(child_process);
 
         const base_args: []const []const u8 = &.{
             options.zig_exe_path,
@@ -475,85 +475,80 @@ pub const BuildOnSave = struct {
         if (options.check_step_only) argv.appendAssumeCapacity("--check-only");
         argv.appendSliceAssumeCapacity(options.build_on_save_args);
 
-        var child_process: std.process.Child = .init(argv.items, options.allocator);
+        child_process.* = .init(argv.items, options.allocator);
         child_process.stdin_behavior = .Pipe;
         child_process.stdout_behavior = .Pipe;
         child_process.stderr_behavior = .Pipe;
         child_process.cwd = options.workspace_path;
 
         child_process.spawn() catch |err| {
+            options.allocator.destroy(child_process);
             log.err("failed to spawn zig build process: {}", .{err});
-            return;
+            return null;
         };
 
         errdefer {
             _ = terminateChildProcessReportError(
-                &child_process,
+                child_process,
                 options.allocator,
                 "zig build runner",
                 .kill,
             );
         }
 
-        self.* = .{
-            .allocator = options.allocator,
-            .child_process = child_process,
-            .thread = undefined, // set below
-        };
-
         const duped_workspace_path = try options.allocator.dupe(u8, options.workspace_path);
         errdefer options.allocator.free(duped_workspace_path);
 
-        self.thread = try std.Thread.spawn(.{ .allocator = options.allocator }, loop, .{
-            self,
+        const thread = try std.Thread.spawn(.{ .allocator = options.allocator }, loop, .{
+            options.allocator,
+            child_process,
             options.collection,
             duped_workspace_path,
         });
+        errdefer comptime unreachable;
+
+        return .{
+            .allocator = options.allocator,
+            .child_process = child_process,
+            .thread = thread,
+        };
     }
 
     pub fn deinit(self: *BuildOnSave) void {
-        // this write tells the child process to exit
-        self.child_process.stdin.?.writeAll("\xaa") catch |err| {
-            if (err != error.BrokenPipe) {
-                log.warn("failed to send message to zig build runner: {}", .{err});
-            }
-            _ = terminateChildProcessReportError(
-                &self.child_process,
-                self.allocator,
-                "zig build runner",
-                .kill,
-            );
-            return;
-        };
+        defer self.* = undefined;
+        defer self.allocator.destroy(self.child_process);
+
+        self.child_process.stdin.?.close();
+        self.child_process.stdin = null;
 
         const success = terminateChildProcessReportError(
-            &self.child_process,
+            self.child_process,
             self.allocator,
             "zig build runner",
             .wait,
         );
         if (!success) return;
 
         self.thread.join();
-        self.* = undefined;
     }
 
     fn loop(
-        self: *BuildOnSave,
+        allocator: std.mem.Allocator,
+        child_process: *std.process.Child,
         collection: *DiagnosticsCollection,
         workspace_path: []const u8,
     ) void {
-        defer self.allocator.free(workspace_path);
+        defer allocator.free(workspace_path);
 
         var transport: Transport = .init(.{
-            .gpa = self.allocator,
-            .in = self.child_process.stdout.?,
-            .out = self.child_process.stdin.?,
+            .gpa = allocator,
+            .in = child_process.stdout.?,
+            .out = child_process.stdin.?,
         });
         defer transport.deinit();
 
         var diagnostic_tags: std.AutoArrayHashMapUnmanaged(DiagnosticsCollection.Tag, void) = .empty;
-        defer diagnostic_tags.deinit(self.allocator);
+        defer diagnostic_tags.deinit(allocator);
 
         defer {
             for (diagnostic_tags.keys()) |tag| collection.clearErrorBundle(tag);
@@ -575,7 +570,7 @@ pub const BuildOnSave = struct {
             switch (@as(ServerToClient.Tag, @enumFromInt(header.tag))) {
                 .watch_error_bundle => {
                     handleWatchErrorBundle(
-                        self,
+                        allocator,
                         &transport,
                         collection,
                         workspace_path,
@@ -593,19 +588,19 @@ pub const BuildOnSave = struct {
     }
 
     fn handleWatchErrorBundle(
-        self: *BuildOnSave,
+        allocator: std.mem.Allocator,
         transport: *Transport,
         collection: *DiagnosticsCollection,
         workspace_path: []const u8,
         diagnostic_tags: *std.AutoArrayHashMapUnmanaged(DiagnosticsCollection.Tag, void),
     ) !void {
         const header = try transport.reader().readStructEndian(ServerToClient.ErrorBundle, .little);
 
-        const extra = try transport.receiveSlice(self.allocator, u32, header.extra_len);
-        defer self.allocator.free(extra);
+        const extra = try transport.receiveSlice(allocator, u32, header.extra_len);
+        defer allocator.free(extra);
 
-        const string_bytes = try transport.receiveBytes(self.allocator, header.string_bytes_len);
-        defer self.allocator.free(string_bytes);
+        const string_bytes = try transport.receiveBytes(allocator, header.string_bytes_len);
+        defer allocator.free(string_bytes);
 
         const error_bundle: std.zig.ErrorBundle = .{ .string_bytes = string_bytes, .extra = extra };
 
@@ -615,7 +610,7 @@ pub const BuildOnSave = struct {
 
         const diagnostic_tag: DiagnosticsCollection.Tag = @enumFromInt(@as(u32, @truncate(hasher.final())));
 
-        try diagnostic_tags.put(self.allocator, diagnostic_tag, {});
+        try diagnostic_tags.put(allocator, diagnostic_tag, {});
 
         try collection.pushErrorBundle(diagnostic_tag, header.cycle, workspace_path, error_bundle);
         try collection.publishDiagnostics();