From 1379bfc2bfc8f96215d9187e72ac30dc5c94225f Mon Sep 17 00:00:00 2001
From: pommicket <pommicket@gmail.com>
Date: Tue, 27 Dec 2022 13:13:49 -0500
Subject: fix LSP thread safety
---
lsp-write.c | 22 +++++++---------------
lsp.c | 32 +++++++++++++++++++++++---------
lsp.h | 57 ++++++++++++++++++++++++++++++++++++++++-----------------
main.c | 2 --
4 files changed, 70 insertions(+), 43 deletions(-)
diff --git a/lsp-write.c b/lsp-write.c
index 93bdd26..f633c2e 100644
--- a/lsp-write.c
+++ b/lsp-write.c
@@ -32,14 +32,6 @@ static const char *lsp_language_id(Language lang) {
return "text";
}
-static const char *lsp_document_path(LSP *lsp, LSPDocumentID document) {
- if (document >= arr_len(lsp->document_data)) {
- assert(0);
- return "";
- }
- return lsp->document_data[document].path;
-}
-
typedef struct {
LSP *lsp;
StrBuilder builder;
@@ -425,15 +417,15 @@ static void write_request(LSP *lsp, LSPRequest *request) {
} break;
case LSP_REQUEST_DID_CHANGE: {
LSPRequestDidChange *change = &request->data.change;
- if (change->document >= arr_len(lsp->document_data)) {
- assert(0);
- break;
- }
- LSPDocumentData *document = &lsp->document_data[change->document];
- ++document->version_number;
+ SDL_LockMutex(lsp->document_mutex);
+ assert(change->document < arr_len(lsp->document_data));
+ LSPDocumentData *document = &lsp->document_data[change->document];
+ u32 version = ++document->version_number;
+ SDL_UnlockMutex(lsp->document_mutex);
+
write_key_obj_start(o, "params");
write_key_obj_start(o, "textDocument");
- write_key_number(o, "version", (double)document->version_number);
+ write_key_number(o, "version", version);
write_key_file_uri(o, "uri", change->document);
write_obj_end(o);
write_key_arr_start(o, "contentChanges");
diff --git a/lsp.c b/lsp.c
index 025cc10..a490aad 100644
--- a/lsp.c
+++ b/lsp.c
@@ -309,15 +309,29 @@ static int lsp_communication_thread(void *data) {
}
u32 lsp_document_id(LSP *lsp, const char *path) {
- u32 *value = str_hash_table_get(&lsp->document_ids, path);
- if (!value) {
- u32 id = arr_len(lsp->document_data);
- value = str_hash_table_insert(&lsp->document_ids, path);
- *value = id;
- LSPDocumentData *data = arr_addp(lsp->document_data);
- data->path = str_dup(path);
- }
- return *value;
+ SDL_LockMutex(lsp->document_mutex);
+ u32 *value = str_hash_table_get(&lsp->document_ids, path);
+ if (!value) {
+ u32 id = arr_len(lsp->document_data);
+ value = str_hash_table_insert(&lsp->document_ids, path);
+ *value = id;
+ LSPDocumentData *data = arr_addp(lsp->document_data);
+ data->path = str_dup(path);
+ }
+ u32 id = *value;
+ SDL_UnlockMutex(lsp->document_mutex);
+ return id;
+}
+
+const char *lsp_document_path(LSP *lsp, LSPDocumentID document) {
+ SDL_LockMutex(lsp->document_mutex);
+ if (document >= arr_len(lsp->document_data)) {
+ assert(0);
+ return "";
+ }
+ const char *path = lsp->document_data[document].path;
+ SDL_UnlockMutex(lsp->document_mutex);
+ return path;
}
LSP *lsp_create(const char *root_dir, Language language, const char *analyzer_command) {
diff --git a/lsp.h b/lsp.h
index c3770bb..7fce761 100644
--- a/lsp.h
+++ b/lsp.h
@@ -271,35 +271,56 @@ typedef struct {
} LSPCapabilities;
typedef struct LSP {
+ // thread safety is important here!
+ // every member should either be indented to indicate which mutex controls it,
+ // or have a comment explaining why it doesn't need one
+
+ // A unique ID number for this LSP.
+ // thread-safety: only set once in lsp_create.
LSPID id;
+
+ // The server process
+ // thread-safety: created in lsp_create, then only accessed by the communication thread
Process process;
+
+ // Which ID number the next request will get
+ // thread-safety: only accessed in communication thread
u32 request_id;
- // for our purposes, folders are "documents"
- // the spec kinda does this too: WorkspaceFolder has a `uri: DocumentUri` member.
- StrHashTable document_ids; // values are u32. they are indices into document_data.
- // this is a dynamic array which just keeps growing.
- // but the user isn't gonna open millions of files so it's fine.
- LSPDocumentData *document_data;
+
+ SDL_mutex *document_mutex;
+ // for our purposes, folders are "documents"
+ // the spec kinda does this too: WorkspaceFolder has a `uri: DocumentUri` member.
+ StrHashTable document_ids; // values are u32. they are indices into document_data.
+ // this is a dynamic array which just keeps growing.
+ // but the user isn't gonna open millions of files so it's fine.
+ LSPDocumentData *document_data;
SDL_mutex *messages_mutex;
- LSPMessage *messages_server2client;
- LSPMessage *messages_client2server;
- // we keep track of client-to-server requests
- // so that we can process responses.
- // this also lets us re-send requests if that's ever necessary.
- LSPRequest *requests_sent;
+ LSPMessage *messages_server2client;
+ LSPMessage *messages_client2server;
+ // we keep track of client-to-server requests
+ // so that we can process responses.
+ // this also lets us re-send requests if that's ever necessary.
+ LSPRequest *requests_sent;
// has the response to the initialize request been sent?
- // we access this both in the main thread and in the LSP communication thread.
+ // thread-safety: this is atomic. it starts out false, and only gets set to true once
+ // (when the initialize response is received)
_Atomic bool initialized;
+ // thread-safety: only set once in lsp_create.
+ Language language;
SDL_Thread *communication_thread;
SDL_sem *quit_sem;
+ // thread-safety: only accessed in communication thread
char *received_data; // dynamic array
+ // thread-safety: in the communication thread, we fill this in, then set `initialized = true`.
+ // after that, this never changes.
+ // never accessed in main thread before `initialized = true`.
LSPCapabilities capabilities;
+ // thread-safety: same as `capabilities`
char32_t *trigger_chars; // dynamic array of "trigger characters"
- SDL_mutex *error_mutex;
- Language language;
SDL_mutex *workspace_folders_mutex;
- LSPDocumentID *workspace_folders; // dynamic array of root directories of LSP workspace folders
- char error[256];
+ LSPDocumentID *workspace_folders; // dynamic array of root directories of LSP workspace folders
+ SDL_mutex *error_mutex;
+ char error[256];
} LSP;
// @TODO: function declarations
@@ -311,6 +332,8 @@ typedef struct LSP {
bool lsp_get_error(LSP *lsp, char *error, size_t error_size, bool clear);
void lsp_message_free(LSPMessage *message);
u32 lsp_document_id(LSP *lsp, const char *path);
+// returned pointer lives exactly as long as lsp.
+const char *lsp_document_path(LSP *lsp, LSPDocumentID id);
// don't free the contents of this request! let me handle it!
void lsp_send_request(LSP *lsp, LSPRequest *request);
// don't free the contents of this response! let me handle it!
diff --git a/main.c b/main.c
index 350e717..a0abf0a 100644
--- a/main.c
+++ b/main.c
@@ -1,7 +1,5 @@
/*
@TODO:
-- lsp_document_id / lsp_document_path thread-safety
-- double check thread safety of other things
- ignore telemetry/event
- https://github.com/typescript-language-server/typescript-language-server
- NOTE: This supports javascript.
--
cgit v1.2.3