Skip to content

Conversation

@AspenJames
Copy link

Fixes file descriptor leak as described in #253

Reproduced by modifying _example/simple-echo/main.go, looping the body of the main function in order to observe multiple calls to prompt.Input(). "Exit" added for convenience:

diff --git a/_example/simple-echo/main.go b/_example/simple-echo/main.go
index 12b1bb4..b02199f 100644
--- a/_example/simple-echo/main.go
+++ b/_example/simple-echo/main.go
@@ -12,17 +12,23 @@ func completer(in prompt.Document) []prompt.Suggest {
 		{Text: "articles", Description: "Store the article text posted by user"},
 		{Text: "comments", Description: "Store the text commented to articles"},
 		{Text: "groups", Description: "Combine users with specific rules"},
+		{Text: "exit", Description: "Stop the loop"},
 	}
 	return prompt.FilterHasPrefix(s, in.GetWordBeforeCursor(), true)
 }
 
 func main() {
-	in := prompt.Input(">>> ", completer,
-		prompt.OptionTitle("sql-prompt"),
-		prompt.OptionHistory([]string{"SELECT * FROM users;"}),
-		prompt.OptionPrefixTextColor(prompt.Yellow),
-		prompt.OptionPreviewSuggestionTextColor(prompt.Blue),
-		prompt.OptionSelectedSuggestionBGColor(prompt.LightGray),
-		prompt.OptionSuggestionBGColor(prompt.DarkGray))
-	fmt.Println("Your input: " + in)
+	for {
+		in := prompt.Input(">>> ", completer,
+			prompt.OptionTitle("sql-prompt"),
+			prompt.OptionHistory([]string{"SELECT * FROM users;"}),
+			prompt.OptionPrefixTextColor(prompt.Yellow),
+			prompt.OptionPreviewSuggestionTextColor(prompt.Blue),
+			prompt.OptionSelectedSuggestionBGColor(prompt.LightGray),
+			prompt.OptionSuggestionBGColor(prompt.DarkGray))
+		if in == "exit" {
+			break
+		}
+		fmt.Println("Your input: " + in)
+	}
 }

Each call to Input will leak another file descriptor, seen with lsof -p <GO PID> | grep 'dev\/tty' | wc -l:

Here you can see the count of /dev/tty file descriptors increasing:
Screen Shot 2022-04-28 at 21 53 46

Calling syscall.Close(t.fd) prevents this leak. After this fix the count of file descriptors is constant:
Screen Shot 2022-04-28 at 21 56 20

@AspenJames AspenJames marked this pull request as ready for review April 29, 2022 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant