From b32718a0135a49cf27c496c83073b15e3e5d33a5 Mon Sep 17 00:00:00 2001 From: TheSmallKiwi Date: Thu, 9 Apr 2026 15:44:03 -0400 Subject: [PATCH] Fix HLSL language server crash on CRLF files pulled in via #include shader-language-server (antaalt/shader-sense) v1.3.0 panics with "byte index N is out of bounds" in symbol_parser.rs when a file is first parsed as an include dependency (raw CRLF bytes via std::fs::read_to_string) and then later opened as a main document. watch_main_file replaces the cached module's content with the client-provided didOpen text but does not re-parse the tree-sitter tree, so if the client normalized line endings (CRLF -> LF) the stored tree references byte offsets past the new content length and the next documentSymbol query slices out of bounds. Python's default text-mode open performs universal-newlines translation, which is what was shrinking the text Serena sent in didOpen. Override open_file in HlslLanguageServer to preload LSPFileBuffer contents with newline="" so the bytes sent to the server match what it reads from disk, sidestepping the stale-tree condition entirely. The fix is scoped to hlsl_language_server.py; no core files are touched. Verified with a 28-shader sweep over a Unity HDRP workspace where the crash was 100% reproducible (TerrainRayMarch.shader -> TerrainSDF.hlsl); all files now return symbols cleanly. The upstream bug should still be fixed in shader-sense (watch_main_file should call update_module rather than assigning .content), but this workaround unblocks Windows users immediately. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../language_servers/hlsl_language_server.py | 97 ++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/src/solidlsp/language_servers/hlsl_language_server.py b/src/solidlsp/language_servers/hlsl_language_server.py index 0a3203eb..2857055b 100644 --- a/src/solidlsp/language_servers/hlsl_language_server.py +++ b/src/solidlsp/language_servers/hlsl_language_server.py @@ -7,13 +7,22 @@ import logging import os import pathlib import shutil +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path from typing import Any, cast import psutil from overrides import override -from solidlsp.ls import LanguageServerDependencyProvider, LanguageServerDependencyProviderSinglePath, SolidLanguageServer +from solidlsp.ls import ( + LanguageServerDependencyProvider, + LanguageServerDependencyProviderSinglePath, + LSPFileBuffer, + SolidLanguageServer, +) from solidlsp.ls_config import LanguageServerConfig +from solidlsp.ls_exceptions import SolidLSPException from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams from solidlsp.settings import SolidLSPSettings @@ -268,6 +277,92 @@ class HlslLanguageServer(SolidLanguageServer): log.debug(f"Error cleaning up shader-language-server process tree: {e}") super().stop(shutdown_timeout) + @contextmanager + def open_file(self, relative_file_path: str, open_in_ls: bool = True) -> Iterator[LSPFileBuffer]: + """Open a file for LSP, preserving on-disk CRLF line endings. + + Workaround for an upstream bug in shader-language-server + (antaalt/shader-sense) where `watch_main_file` replaces an already-cached + module's content without re-parsing the tree-sitter tree. When a file is + first pulled into the server's cache via an `#include` from another + shader (where it's read via `std::fs::read_to_string`, preserving CRLF), + and then later opened directly via `textDocument/didOpen` with the + client-normalized LF text, the stored tree still references byte offsets + into the longer CRLF content. The next symbol query slices the new + (shorter) content with stale offsets and panics with + `byte index N is out of bounds` in `shader-sense/src/symbols/symbol_parser.rs`. + + The root-cause fix belongs upstream (the server should call + `update_module` instead of assigning `content` raw). Until then, we + ensure the text we send in `didOpen` matches byte-for-byte what the + server reads from disk by preloading the file buffer with a + CRLF-preserving read before the LSP notification is sent. + + This is the only place in Serena that overrides `open_file`; the fix is + deliberately scoped to the HLSL language server. It mirrors the base + class logic in `SolidLanguageServer.open_file` verbatim except for the + buffer construction branch, where creation is deferred (`open_in_ls=False`) + so the buffer's contents can be preloaded before `ensure_open_in_ls` runs. + """ + if not self.server_started: + log.error("open_file called before Language Server started") + raise SolidLSPException("Language Server not started") + + absolute_file_path = Path(self.repository_root_path, relative_file_path) + uri = absolute_file_path.as_uri() + + if uri in self.open_file_buffers: + fb = self.open_file_buffers[uri] + assert fb.uri == uri + assert fb.ref_count >= 1 + + fb.ref_count += 1 + if open_in_ls: + fb.ensure_open_in_ls() + yield fb + fb.ref_count -= 1 + else: + version = 0 + language_id = self._get_language_id_for_file(relative_file_path) + # Defer the didOpen so we can preload CRLF-preserved content first. + fb = LSPFileBuffer( + abs_path=absolute_file_path, + uri=uri, + encoding=self._encoding, + version=version, + language_id=language_id, + ref_count=1, + language_server=self, + open_in_ls=False, + ) + self._preload_crlf_content(fb) + self.open_file_buffers[uri] = fb + if open_in_ls: + fb.ensure_open_in_ls() + yield fb + fb.ref_count -= 1 + + if self.open_file_buffers[uri].ref_count == 0: + self.open_file_buffers[uri].close() + del self.open_file_buffers[uri] + + def _preload_crlf_content(self, fb: LSPFileBuffer) -> None: + """Populate an LSPFileBuffer with a CRLF-preserving read of its backing file. + + Python's default text-mode open applies universal-newlines translation + (CRLF -> LF), which would desync the client's `didOpen` text from the + server-side `std::fs::read_to_string` view that parsed the dependency + tree. Passing `newline=""` disables the translation so bytes match. + """ + with open(fb.abs_path, encoding=fb.encoding, newline="") as f: + raw = f.read() + # Set the buffer's cached state directly: the contents, the mtime + # (required by the contents property's staleness check), and clear the + # hash so it's recomputed against the new bytes. + fb._contents = raw + fb._read_file_modified_date = fb.abs_path.stat().st_mtime + fb._content_hash = None + @override def is_ignored_dirname(self, dirname: str) -> bool: """Ignore Unity-specific directories that contain no user-authored shaders."""