fix: remove multithreaded buffer access in project_manager

Until we have proper multithreaded buffer lifetime management we should avoid
accessing buffers that may have been deleted already.
This commit is contained in:
CJ van den Berg 2025-06-03 18:14:00 +02:00
parent 3853ac8aea
commit 006e1ddb45
Signed by: neurocyte
GPG key ID: 8EB1E1BB660E3FB9
3 changed files with 40 additions and 33 deletions

View file

@ -520,6 +520,7 @@ pub fn delete_task(self: *Self, command: []const u8) error{}!void {
} }
pub fn did_open(self: *Self, file_path: []const u8, file_type: []const u8, language_server: []const u8, version: usize, text: []const u8) StartLspError!void { pub fn did_open(self: *Self, file_path: []const u8, file_type: []const u8, language_server: []const u8, version: usize, text: []const u8) StartLspError!void {
defer std.heap.c_allocator.free(text);
self.update_mru(file_path, 0, 0) catch {}; self.update_mru(file_path, 0, 0) catch {};
const lsp = try self.get_or_start_language_server(file_path, language_server); const lsp = try self.get_or_start_language_server(file_path, language_server);
const uri = try self.make_URI(file_path); const uri = try self.make_URI(file_path);
@ -529,7 +530,10 @@ pub fn did_open(self: *Self, file_path: []const u8, file_type: []const u8, langu
}) catch return error.LspFailed; }) catch return error.LspFailed;
} }
pub fn did_change(self: *Self, file_path: []const u8, version: usize, root_dst_addr: usize, root_src_addr: usize, eol_mode: Buffer.EolMode) LspError!void { pub fn did_change(self: *Self, file_path: []const u8, version: usize, text_dst: []const u8, text_src: []const u8, eol_mode: Buffer.EolMode) LspError!void {
_ = eol_mode;
defer std.heap.c_allocator.free(text_dst);
defer std.heap.c_allocator.free(text_src);
const lsp = try self.get_language_server(file_path); const lsp = try self.get_language_server(file_path);
const uri = try self.make_URI(file_path); const uri = try self.make_URI(file_path);
@ -545,22 +549,11 @@ pub fn did_change(self: *Self, file_path: []const u8, version: usize, root_dst_a
self.allocator.free(scratch); self.allocator.free(scratch);
} }
const root_dst: Buffer.Root = if (root_dst_addr == 0) return else @ptrFromInt(root_dst_addr);
const root_src: Buffer.Root = if (root_src_addr == 0) return else @ptrFromInt(root_src_addr);
var dizzy_edits = std.ArrayListUnmanaged(dizzy.Edit){}; var dizzy_edits = std.ArrayListUnmanaged(dizzy.Edit){};
var dst = std.ArrayList(u8).init(arena);
var src = std.ArrayList(u8).init(arena);
var edits_cb = std.ArrayList(u8).init(arena); var edits_cb = std.ArrayList(u8).init(arena);
const writer = edits_cb.writer(); const writer = edits_cb.writer();
{ const scratch_len = 4 * (text_dst.len + text_src.len) + 2;
const frame = tracy.initZone(@src(), .{ .name = "store" });
defer frame.deinit();
try root_dst.store(dst.writer(), eol_mode);
try root_src.store(src.writer(), eol_mode);
}
const scratch_len = 4 * (dst.items.len + src.items.len) + 2;
const scratch = blk: { const scratch = blk: {
const frame = tracy.initZone(@src(), .{ .name = "scratch" }); const frame = tracy.initZone(@src(), .{ .name = "scratch" });
defer frame.deinit(); defer frame.deinit();
@ -571,7 +564,7 @@ pub fn did_change(self: *Self, file_path: []const u8, version: usize, root_dst_a
{ {
const frame = tracy.initZone(@src(), .{ .name = "diff" }); const frame = tracy.initZone(@src(), .{ .name = "diff" });
defer frame.deinit(); defer frame.deinit();
try dizzy.PrimitiveSliceDiffer(u8).diff(arena, &dizzy_edits, src.items, dst.items, scratch); try dizzy.PrimitiveSliceDiffer(u8).diff(arena, &dizzy_edits, text_src, text_dst, scratch);
} }
var lines_dst: usize = 0; var lines_dst: usize = 0;
var last_offset: usize = 0; var last_offset: usize = 0;
@ -583,7 +576,7 @@ pub fn did_change(self: *Self, file_path: []const u8, version: usize, root_dst_a
for (dizzy_edits.items) |dizzy_edit| { for (dizzy_edits.items) |dizzy_edit| {
switch (dizzy_edit.kind) { switch (dizzy_edit.kind) {
.equal => { .equal => {
scan_char(src.items[dizzy_edit.range.start..dizzy_edit.range.end], &lines_dst, '\n', &last_offset); scan_char(text_src[dizzy_edit.range.start..dizzy_edit.range.end], &lines_dst, '\n', &last_offset);
}, },
.insert => { .insert => {
const line_start_dst: usize = lines_dst; const line_start_dst: usize = lines_dst;
@ -592,15 +585,15 @@ pub fn did_change(self: *Self, file_path: []const u8, version: usize, root_dst_a
.start = .{ .line = line_start_dst, .character = last_offset }, .start = .{ .line = line_start_dst, .character = last_offset },
.end = .{ .line = line_start_dst, .character = last_offset }, .end = .{ .line = line_start_dst, .character = last_offset },
}, },
.text = dst.items[dizzy_edit.range.start..dizzy_edit.range.end], .text = text_dst[dizzy_edit.range.start..dizzy_edit.range.end],
}); });
edits_count += 1; edits_count += 1;
scan_char(dst.items[dizzy_edit.range.start..dizzy_edit.range.end], &lines_dst, '\n', &last_offset); scan_char(text_dst[dizzy_edit.range.start..dizzy_edit.range.end], &lines_dst, '\n', &last_offset);
}, },
.delete => { .delete => {
var line_end_dst: usize = lines_dst; var line_end_dst: usize = lines_dst;
var offset_end_dst: usize = last_offset; var offset_end_dst: usize = last_offset;
scan_char(src.items[dizzy_edit.range.start..dizzy_edit.range.end], &line_end_dst, '\n', &offset_end_dst); scan_char(text_src[dizzy_edit.range.start..dizzy_edit.range.end], &line_end_dst, '\n', &offset_end_dst);
try cbor.writeValue(writer, .{ try cbor.writeValue(writer, .{
.range = .{ .range = .{
.start = .{ .line = lines_dst, .character = last_offset }, .start = .{ .line = lines_dst, .character = last_offset },

View file

@ -147,11 +147,13 @@ pub fn did_open(file_path: []const u8, file_type: *const FileType, version: usiz
return send(.{ "did_open", project, file_path, file_type.name, language_server, version, text_ptr, text.len }); return send(.{ "did_open", project, file_path, file_type.name, language_server, version, text_ptr, text.len });
} }
pub fn did_change(file_path: []const u8, version: usize, root_dst: usize, root_src: usize, eol_mode: Buffer.EolMode) (ProjectManagerError || ProjectError)!void { pub fn did_change(file_path: []const u8, version: usize, text_dst: []const u8, text_src: []const u8, eol_mode: Buffer.EolMode) (ProjectManagerError || ProjectError)!void {
const project = tp.env.get().str("project"); const project = tp.env.get().str("project");
if (project.len == 0) if (project.len == 0)
return error.NoProject; return error.NoProject;
return send(.{ "did_change", project, file_path, version, root_dst, root_src, @intFromEnum(eol_mode) }); const text_dst_ptr: usize = if (text_dst.len > 0) @intFromPtr(text_dst.ptr) else 0;
const text_src_ptr: usize = if (text_src.len > 0) @intFromPtr(text_src.ptr) else 0;
return send(.{ "did_change", project, file_path, version, text_dst_ptr, text_dst.len, text_src_ptr, text_src.len, @intFromEnum(eol_mode) });
} }
pub fn did_save(file_path: []const u8) (ProjectManagerError || ProjectError)!void { pub fn did_save(file_path: []const u8) (ProjectManagerError || ProjectError)!void {
@ -322,12 +324,14 @@ const Process = struct {
var version: usize = 0; var version: usize = 0;
var text_ptr: usize = 0; var text_ptr: usize = 0;
var text_len: usize = 0; var text_len: usize = 0;
var text_dst_ptr: usize = 0;
var text_dst_len: usize = 0;
var text_src_ptr: usize = 0;
var text_src_len: usize = 0;
var n: usize = 0; var n: usize = 0;
var task: []const u8 = undefined; var task: []const u8 = undefined;
var context: usize = undefined; var context: usize = undefined;
var root_dst: usize = 0;
var root_src: usize = 0;
var eol_mode: Buffer.EolModeTag = @intFromEnum(Buffer.EolMode.lf); var eol_mode: Buffer.EolModeTag = @intFromEnum(Buffer.EolMode.lf);
if (try cbor.match(m.buf, .{ "walk_tree_entry", tp.extract(&project_directory), tp.extract(&path), tp.extract(&high), tp.extract(&low) })) { if (try cbor.match(m.buf, .{ "walk_tree_entry", tp.extract(&project_directory), tp.extract(&path), tp.extract(&high), tp.extract(&low) })) {
@ -373,8 +377,10 @@ const Process = struct {
} else if (try cbor.match(m.buf, .{ "did_open", tp.extract(&project_directory), tp.extract(&path), tp.extract(&file_type), tp.extract_cbor(&language_server), tp.extract(&version), tp.extract(&text_ptr), tp.extract(&text_len) })) { } else if (try cbor.match(m.buf, .{ "did_open", tp.extract(&project_directory), tp.extract(&path), tp.extract(&file_type), tp.extract_cbor(&language_server), tp.extract(&version), tp.extract(&text_ptr), tp.extract(&text_len) })) {
const text = if (text_len > 0) @as([*]const u8, @ptrFromInt(text_ptr))[0..text_len] else ""; const text = if (text_len > 0) @as([*]const u8, @ptrFromInt(text_ptr))[0..text_len] else "";
self.did_open(project_directory, path, file_type, language_server, version, text) catch |e| return from.forward_error(e, @errorReturnTrace()) catch error.ClientFailed; self.did_open(project_directory, path, file_type, language_server, version, text) catch |e| return from.forward_error(e, @errorReturnTrace()) catch error.ClientFailed;
} else if (try cbor.match(m.buf, .{ "did_change", tp.extract(&project_directory), tp.extract(&path), tp.extract(&version), tp.extract(&root_dst), tp.extract(&root_src), tp.extract(&eol_mode) })) { } else if (try cbor.match(m.buf, .{ "did_change", tp.extract(&project_directory), tp.extract(&path), tp.extract(&version), tp.extract(&text_dst_ptr), tp.extract(&text_dst_len), tp.extract(&text_src_ptr), tp.extract(&text_src_len), tp.extract(&eol_mode) })) {
self.did_change(project_directory, path, version, root_dst, root_src, @enumFromInt(eol_mode)) catch |e| return from.forward_error(e, @errorReturnTrace()) catch error.ClientFailed; const text_dst = if (text_dst_len > 0) @as([*]const u8, @ptrFromInt(text_dst_ptr))[0..text_dst_len] else "";
const text_src = if (text_src_len > 0) @as([*]const u8, @ptrFromInt(text_src_ptr))[0..text_src_len] else "";
self.did_change(project_directory, path, version, text_dst, text_src, @enumFromInt(eol_mode)) catch |e| return from.forward_error(e, @errorReturnTrace()) catch error.ClientFailed;
} else if (try cbor.match(m.buf, .{ "did_save", tp.extract(&project_directory), tp.extract(&path) })) { } else if (try cbor.match(m.buf, .{ "did_save", tp.extract(&project_directory), tp.extract(&path) })) {
self.did_save(project_directory, path) catch |e| return from.forward_error(e, @errorReturnTrace()) catch error.ClientFailed; self.did_save(project_directory, path) catch |e| return from.forward_error(e, @errorReturnTrace()) catch error.ClientFailed;
} else if (try cbor.match(m.buf, .{ "did_close", tp.extract(&project_directory), tp.extract(&path) })) { } else if (try cbor.match(m.buf, .{ "did_close", tp.extract(&project_directory), tp.extract(&path) })) {
@ -497,11 +503,11 @@ const Process = struct {
return project.did_open(file_path, file_type, language_server, version, text); return project.did_open(file_path, file_type, language_server, version, text);
} }
fn did_change(self: *Process, project_directory: []const u8, file_path: []const u8, version: usize, root_dst: usize, root_src: usize, eol_mode: Buffer.EolMode) (ProjectError || Project.LspError)!void { fn did_change(self: *Process, project_directory: []const u8, file_path: []const u8, version: usize, text_dst: []const u8, text_src: []const u8, eol_mode: Buffer.EolMode) (ProjectError || Project.LspError)!void {
const frame = tracy.initZone(@src(), .{ .name = module_name ++ ".did_change" }); const frame = tracy.initZone(@src(), .{ .name = module_name ++ ".did_change" });
defer frame.deinit(); defer frame.deinit();
const project = self.projects.get(project_directory) orelse return error.NoProject; const project = self.projects.get(project_directory) orelse return error.NoProject;
return project.did_change(file_path, version, root_dst, root_src, eol_mode); return project.did_change(file_path, version, text_dst, text_src, eol_mode);
} }
fn did_save(self: *Process, project_directory: []const u8, file_path: []const u8) (ProjectError || Project.LspError)!void { fn did_save(self: *Process, project_directory: []const u8, file_path: []const u8) (ProjectError || Project.LspError)!void {

View file

@ -571,11 +571,11 @@ pub const Editor = struct {
self.syntax = syntax: { self.syntax = syntax: {
const lang_override = file_type orelse tp.env.get().str("language"); const lang_override = file_type orelse tp.env.get().str("language");
var content = std.ArrayListUnmanaged(u8).empty; var content = std.ArrayListUnmanaged(u8).empty;
defer content.deinit(self.allocator); defer content.deinit(std.heap.c_allocator);
{ {
const frame_ = tracy.initZone(@src(), .{ .name = "store" }); const frame_ = tracy.initZone(@src(), .{ .name = "store" });
defer frame_.deinit(); defer frame_.deinit();
try new_buf.root.store(content.writer(self.allocator), new_buf.file_eol_mode); try new_buf.root.store(content.writer(std.heap.c_allocator), new_buf.file_eol_mode);
} }
const syn_file_type = blk: { const syn_file_type = blk: {
@ -603,7 +603,7 @@ pub const Editor = struct {
file_path, file_path,
syn_.file_type, syn_.file_type,
self.lsp_version, self.lsp_version,
try content.toOwnedSlice(self.allocator), try content.toOwnedSlice(std.heap.c_allocator),
new_buf.is_ephemeral(), new_buf.is_ephemeral(),
) catch |e| ) catch |e|
self.logger.print("project_manager.did_open failed: {any}", .{e}); self.logger.print("project_manager.did_open failed: {any}", .{e});
@ -1669,10 +1669,18 @@ pub const Editor = struct {
return if (p) |p_| @intFromPtr(p_) else 0; return if (p) |p_| @intFromPtr(p_) else 0;
} }
fn text_from_root(root_: ?Buffer.Root, eol_mode: Buffer.EolMode) ![]const u8 {
const root = root_ orelse return &.{};
var text = std.ArrayList(u8).init(std.heap.c_allocator);
defer text.deinit();
try root.store(text.writer(), eol_mode);
return text.toOwnedSlice();
}
fn send_editor_update(self: *const Self, old_root: ?Buffer.Root, new_root: ?Buffer.Root, eol_mode: Buffer.EolMode) !void { fn send_editor_update(self: *const Self, old_root: ?Buffer.Root, new_root: ?Buffer.Root, eol_mode: Buffer.EolMode) !void {
_ = try self.handlers.msg(.{ "E", "update", token_from(new_root), token_from(old_root), @intFromEnum(eol_mode) }); _ = try self.handlers.msg(.{ "E", "update", token_from(new_root), token_from(old_root), @intFromEnum(eol_mode) });
if (self.syntax) |_| if (self.file_path) |file_path| if (old_root != null and new_root != null) if (self.syntax) |_| if (self.file_path) |file_path| if (old_root != null and new_root != null)
project_manager.did_change(file_path, self.lsp_version, token_from(new_root), token_from(old_root), eol_mode) catch {}; project_manager.did_change(file_path, self.lsp_version, try text_from_root(new_root, eol_mode), try text_from_root(old_root, eol_mode), eol_mode) catch {};
} }
fn send_editor_eol_mode(self: *const Self, eol_mode: Buffer.EolMode, utf8_sanitized: bool) !void { fn send_editor_eol_mode(self: *const Self, eol_mode: Buffer.EolMode, utf8_sanitized: bool) !void {
@ -5809,16 +5817,16 @@ pub const Editor = struct {
self.syntax = syntax: { self.syntax = syntax: {
var content = std.ArrayListUnmanaged(u8).empty; var content = std.ArrayListUnmanaged(u8).empty;
defer content.deinit(self.allocator); defer content.deinit(std.heap.c_allocator);
const root = try self.buf_root(); const root = try self.buf_root();
try root.store(content.writer(self.allocator), try self.buf_eol_mode()); try root.store(content.writer(std.heap.c_allocator), try self.buf_eol_mode());
const syn = syntax.create_file_type(self.allocator, file_type, tui.query_cache()) catch null; const syn = syntax.create_file_type(self.allocator, file_type, tui.query_cache()) catch null;
if (syn) |syn_| if (self.file_path) |file_path| if (syn) |syn_| if (self.file_path) |file_path|
project_manager.did_open( project_manager.did_open(
file_path, file_path,
syn_.file_type, syn_.file_type,
self.lsp_version, self.lsp_version,
try content.toOwnedSlice(self.allocator), try content.toOwnedSlice(std.heap.c_allocator),
if (self.buffer) |p| p.is_ephemeral() else true, if (self.buffer) |p| p.is_ephemeral() else true,
) catch |e| ) catch |e|
self.logger.print("project_manager.did_open failed: {any}", .{e}); self.logger.print("project_manager.did_open failed: {any}", .{e});