From 923dbd5d317a2b6ca5f6520297fab9b4d8148221 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Mar 2023 16:01:17 +0100 Subject: [PATCH 1/5] Create a detached process to clean up tmp dirs on closing The temp dir cleanup is done on the "exit" notification. BTW, if the OS/system is too slow, the language server may be killed by the IDE if it takes too long to clean up temporary build directories. To avoid it a new detached process is created with the only purpose to remove the temporary directories. Since it's detached it will run even after the parent process exits. --- ls/ls.go | 50 ++++++++++++++++++++++++++++++++++++++++++-------- main.go | 7 +++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/ls/ls.go b/ls/ls.go index 17e4f56..ea2933d 100644 --- a/ls/ls.go +++ b/ls/ls.go @@ -22,6 +22,7 @@ import ( "io" "log" "os" + "os/exec" "strconv" "strings" "sync" @@ -50,6 +51,7 @@ type INOLanguageServer struct { progressHandler *progressProxyHandler closing chan bool + removeTempMutex sync.Mutex clangdStarted *sync.Cond dataMux sync.RWMutex buildPath *paths.Path @@ -387,6 +389,7 @@ func (ls *INOLanguageServer) shutdownReqFromIDE(ctx context.Context, logger json close(done) }() _, _ = ls.Clangd.conn.Shutdown(context.Background()) + ls.removeTemporaryFiles(logger) <-done return nil } @@ -1371,6 +1374,45 @@ func (ls *INOLanguageServer) setTraceNotifFromIDE(logger jsonrpc.FunctionLogger, ls.Clangd.conn.SetTrace(params) } +func (ls *INOLanguageServer) removeTemporaryFiles(logger jsonrpc.FunctionLogger) { + ls.removeTempMutex.Lock() + defer ls.removeTempMutex.Unlock() + + args := []string{"remove-temp-files"} + if ls.buildPath != nil { + args = append(args, ls.buildPath.String()) + } + if ls.fullBuildPath != nil { + args = append(args, ls.fullBuildPath.String()) + } + if len(args) == 1 { + // Nothing to remove + return + } + + // Start a detached process to remove the temp files + cwd, err := os.Getwd() + if err != nil { + logger.Logf("Error getting current working directory: %s", err) + return + } + cmd := exec.Command(os.Args[0], args...) + cmd.Dir = cwd + if err := cmd.Start(); err != nil { + logger.Logf("Error starting remove-temp-files process: %s", err) + return + } + + // The process is now started, we can reset the paths + ls.buildPath, ls.fullBuildPath = nil, nil + + // Detach the process so it can continue running even if the parent process exits + if err := cmd.Process.Release(); err != nil { + logger.Logf("Error detaching remove-temp-files process: %s", err) + return + } +} + // Close closes all the json-rpc connections and clean-up temp folders. func (ls *INOLanguageServer) Close() { if ls.Clangd != nil { @@ -1381,14 +1423,6 @@ func (ls *INOLanguageServer) Close() { close(ls.closing) ls.closing = nil } - if ls.buildPath != nil { - ls.buildPath.RemoveAll() - ls.buildPath = nil - } - if ls.fullBuildPath != nil { - ls.fullBuildPath.RemoveAll() - ls.fullBuildPath = nil - } } // CloseNotify returns a channel that is closed when the InoHandler is closed diff --git a/main.go b/main.go index 3333672..0143adc 100644 --- a/main.go +++ b/main.go @@ -20,6 +20,13 @@ import ( ) func main() { + if len(os.Args) > 1 && os.Args[1] == "remove-temp-files" { + for _, tmpFile := range os.Args[2:] { + paths.New(tmpFile).RemoveAll() + } + return + } + clangdPath := flag.String( "clangd", "", "Path to clangd executable") From 11be75009deb32f75301232b1a9c0347b9f5fc77 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Mar 2023 16:19:20 +0100 Subject: [PATCH 2/5] Keep all language-server tmp files inside a single subdir --- ls/ls.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/ls/ls.go b/ls/ls.go index ea2933d..f5d734a 100644 --- a/ls/ls.go +++ b/ls/ls.go @@ -54,6 +54,7 @@ type INOLanguageServer struct { removeTempMutex sync.Mutex clangdStarted *sync.Cond dataMux sync.RWMutex + tempDir *paths.Path buildPath *paths.Path buildSketchRoot *paths.Path buildSketchCpp *paths.Path @@ -146,18 +147,21 @@ func NewINOLanguageServer(stdin io.Reader, stdout io.Writer, config *Config) *IN if tmp, err := paths.MkTempDir("", "arduino-language-server"); err != nil { log.Fatalf("Could not create temp folder: %s", err) } else { - ls.buildPath = tmp.Canonical() - ls.buildSketchRoot = ls.buildPath.Join("sketch") + ls.tempDir = tmp.Canonical() } - - if tmp, err := paths.MkTempDir("", "arduino-language-server"); err != nil { + ls.buildPath = ls.tempDir.Join("build") + ls.buildSketchRoot = ls.buildPath.Join("sketch") + if err := ls.buildPath.MkdirAll(); err != nil { + log.Fatalf("Could not create temp folder: %s", err) + } + ls.fullBuildPath = ls.tempDir.Join("fullbuild") + if err := ls.fullBuildPath.MkdirAll(); err != nil { log.Fatalf("Could not create temp folder: %s", err) - } else { - ls.fullBuildPath = tmp.Canonical() } logger.Logf("Initial board configuration: %s", ls.config.Fqbn) logger.Logf("%s", globals.VersionInfo.String()) + logger.Logf("Language server temp directory: %s", ls.tempDir) logger.Logf("Language server build path: %s", ls.buildPath) logger.Logf("Language server build sketch root: %s", ls.buildSketchRoot) logger.Logf("Language server FULL build path: %s", ls.fullBuildPath) @@ -1378,14 +1382,7 @@ func (ls *INOLanguageServer) removeTemporaryFiles(logger jsonrpc.FunctionLogger) ls.removeTempMutex.Lock() defer ls.removeTempMutex.Unlock() - args := []string{"remove-temp-files"} - if ls.buildPath != nil { - args = append(args, ls.buildPath.String()) - } - if ls.fullBuildPath != nil { - args = append(args, ls.fullBuildPath.String()) - } - if len(args) == 1 { + if ls.tempDir == nil { // Nothing to remove return } @@ -1396,7 +1393,7 @@ func (ls *INOLanguageServer) removeTemporaryFiles(logger jsonrpc.FunctionLogger) logger.Logf("Error getting current working directory: %s", err) return } - cmd := exec.Command(os.Args[0], args...) + cmd := exec.Command(os.Args[0], "remove-temp-files", ls.tempDir.String()) cmd.Dir = cwd if err := cmd.Start(); err != nil { logger.Logf("Error starting remove-temp-files process: %s", err) @@ -1404,7 +1401,7 @@ func (ls *INOLanguageServer) removeTemporaryFiles(logger jsonrpc.FunctionLogger) } // The process is now started, we can reset the paths - ls.buildPath, ls.fullBuildPath = nil, nil + ls.buildPath, ls.fullBuildPath, ls.buildSketchRoot, ls.tempDir = nil, nil, nil, nil // Detach the process so it can continue running even if the parent process exits if err := cmd.Process.Release(); err != nil { From ffb735c793907870f8dea6ad9bf683f9a1a5cc27 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Mar 2023 16:20:04 +0100 Subject: [PATCH 3/5] Tell clangd to write tmp files inside the language server tmp dir Fix #145 --- ls/lsp_client_clangd.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ls/lsp_client_clangd.go b/ls/lsp_client_clangd.go index 83789bb..d4b9322 100644 --- a/ls/lsp_client_clangd.go +++ b/ls/lsp_client_clangd.go @@ -60,7 +60,12 @@ func newClangdLSPClient(logger jsonrpc.FunctionLogger, dataFolder *paths.Path, l logger.Logf(" Starting clangd: %s", strings.Join(args, " ")) var clangdStdin io.WriteCloser var clangdStdout, clangdStderr io.ReadCloser - if clangdCmd, err := executils.NewProcess(nil, args...); err != nil { + var extraEnv []string + if ls.tempDir != nil { + extraEnv = append(extraEnv, "TMPDIR="+ls.tempDir.String()) // For unix-based systems + extraEnv = append(extraEnv, "TMP="+ls.tempDir.String()) // For Windows + } + if clangdCmd, err := executils.NewProcess(extraEnv, args...); err != nil { panic("starting clangd: " + err.Error()) } else if cin, err := clangdCmd.StdinPipe(); err != nil { panic("getting clangd stdin: " + err.Error()) From 84f3847b6a06fb9a77a198d811a8734d642abefd Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Mar 2023 16:20:33 +0100 Subject: [PATCH 4/5] Fixed an unrelated linter error --- ls/lsp_server_ide.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/ls/lsp_server_ide.go b/ls/lsp_server_ide.go index 5fbe3b5..28228cf 100644 --- a/ls/lsp_server_ide.go +++ b/ls/lsp_server_ide.go @@ -303,8 +303,6 @@ func (server *IDELSPServer) WorkspaceDidChangeConfiguration(logger jsonrpc.Funct // // Since ALS doesn’t have any workspace configuration yet, // ignore it. - return - } // WorkspaceDidChangeWatchedFiles is not implemented From ca7d9ae34f75f30b6f7854daf108a47ec1625ced Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Aug 2023 14:48:55 +0200 Subject: [PATCH 5/5] Added safety check --- main.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/main.go b/main.go index 0143adc..ed38ad5 100644 --- a/main.go +++ b/main.go @@ -12,6 +12,7 @@ import ( "os/signal" "os/user" "path" + "strings" "github.com/arduino/arduino-language-server/ls" "github.com/arduino/arduino-language-server/streams" @@ -22,6 +23,12 @@ import ( func main() { if len(os.Args) > 1 && os.Args[1] == "remove-temp-files" { for _, tmpFile := range os.Args[2:] { + // SAFETY CHECK + if !strings.Contains(tmpFile, "arduino-language-server") { + fmt.Println("Could not remove extraneous temp folder:", tmpFile) + os.Exit(1) + } + paths.New(tmpFile).RemoveAll() } return