Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce likelihood of races between stdout and cooked stdin reads #18326

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Dec 13, 2024

As explained in the comment on _getViewportCursorPosition, printing
to stdout after initiating a cooked stdin reads is a race condition
between the application and the terminal. But we can significantly
reduce the likelihood of this being obvious with this change.

Related to #18265
Possibly related to #18081

Validation Steps Performed

Execute the following Go code and start typing:

package main

import (
	"fmt"
	"time"
)

func main() {
	go func() {
		time.Sleep(50 * time.Millisecond)
		fmt.Printf("Here is a prompt! >")
	}()

	var text string
	fmt.Scanln(&text)
}

Without this change the prompt will disappear,
and with this change in place, it'll work as expected. ✅

@lhecker lhecker added the Area-CookedRead The cmd.exe COOKED_READ handling label Dec 13, 2024
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one COOKED_READ_DATA per read, right? We never have to consider invalidating this new member?

Thanks for doing this!

@lhecker
Copy link
Member Author

lhecker commented Jan 24, 2025

Yes exactly.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 😊

@carlos-zamora carlos-zamora merged commit 1040035 into main Jan 27, 2025
20 checks passed
@carlos-zamora carlos-zamora deleted the dev/lhecker/18265-cooked-delay branch January 27, 2025 22:37
DHowett pushed a commit that referenced this pull request Jan 28, 2025
)

As explained in the comment on `_getViewportCursorPosition`, printing
to stdout after initiating a cooked stdin reads is a race condition
between the application and the terminal. But we can significantly
reduce the likelihood of this being obvious with this change.

Related to #18265
Possibly related to #18081

## Validation Steps Performed

Execute the following Go code and start typing:
```go
package main

import (
	"fmt"
	"time"
)

func main() {
	go func() {
		time.Sleep(50 * time.Millisecond)
		fmt.Printf("Here is a prompt! >")
	}()

	var text string
	fmt.Scanln(&text)
}
```

Without this change the prompt will disappear,
and with this change in place, it'll work as expected. ✅

(cherry picked from commit 1040035)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgWppag
Service-Version: 1.22
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-CookedRead The cmd.exe COOKED_READ handling
Projects
Status: Cherry Picked
Development

Successfully merging this pull request may close these issues.

3 participants