Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

OS.execute_with_pipe is very buggy #102340

Closed
RoyalXXX opened this issue Feb 2, 2025 · 4 comments · Fixed by #102365
Closed

OS.execute_with_pipe is very buggy #102340

RoyalXXX opened this issue Feb 2, 2025 · 4 comments · Fixed by #102365
Assignees
Milestone

Comments

@RoyalXXX
Copy link

RoyalXXX commented Feb 2, 2025

Tested versions

Godot 4.3 latest stable

System information

Win 11 64 bit

Issue description

I encountered an issue with OS.execute_with_pipe while trying to redirect the standard input and output of another process. I used this function to obtain a FileAccess handle for the process's stdio.

However, it turns out that the FileAccess instance created via OS.execute_with_pipe has significant limitations compared to a FileAccess instance created from a file. Specifically, the following functions do not work at all:

  • get_as_text()
  • eof_reached()
  • get_length()
  • get_position()

This makes reading output from the redirected process problematic. The get_line() function works, but if I attempt to read more data than is available in the output, the program crashes.

I could not find a single reliable way to determine whether there is data available to read before attempting to do so. As a result, working with pipes in Godot becomes nearly impossible.

How should I properly retrieve text from a redirected process in this case?

Steps to reproduce

...

Minimal reproduction project (MRP)

...

@bruvzg
Copy link
Member

bruvzg commented Feb 2, 2025

get_as_text()
eof_reached()
get_length()
get_position()

This is fully expected, none of these functions make any sense for a pipe. Pipe is intended to continuously stream data (in both directions), so there's no position, size or defined end.

All you can do is try reading until it fails (no more data in the input buffer), e.g.:

if pipe["stdio"].is_open():
	var buffer = PackedByteArray()
	while true:
		buffer.append_array(pipe["stdio"].get_buffer(2048))
		if pipe["stdio"].get_error() != OK:
			break;
	#do sometheing with data buffer, and repeate to det more data.

If you simply need full output, use OS.execute instead.

The get_line() function works, but if I attempt to read more data than is available in the output, the program crashes.

I'll check it, crashing is definitely a bug, it should return an error if no data is available, but likely keep waiting in the infinite loop until \n is received.

@RoyalXXX
Copy link
Author

RoyalXXX commented Feb 2, 2025

If you simply need full output, use OS.execute instead.

I need to send and receive text between two processes, similar to how a chess GUI communicates with a chess engine. I have a console application written in C++, and my game in Godot needs to exchange information with it continuously. A one-time output retrieval is not sufficient for my case.

All you can do is try reading until it fails (no more data in the input buffer), e.g.:

The proposed method does allow reading the data completely, but if I try to run this code when the output is empty, it still crashes. Am I correct in understanding that there is no way to check if the output in the pipe is empty? So, it turns out that I need to start reading only when I’m sure that there is something there.

@bruvzg
Copy link
Member

bruvzg commented Feb 2, 2025

Am I correct in understanding that there is no way to check if the output in the pipe is empty?

Note that pipes were reworked for 4.4 (#94434), so my example is not correct for 4.3.

In 4.4, trying to read and checking get_error() after to check if is the read was successful is the intended way to check if it's empty.

In 4.3 it is more limited and always blocking, pipe will wait until data is received or pipe is closed. So in 4.3 the only way is to use it from the thread:

...
	thread = Thread.new()
	thread.start(_thread_func)
	get_window().close_requested.connect(clean_func)


func _thread_func():
	# read stdin and add to TextEdit.
	while pipe.is_open() and pipe.get_error() == OK:
		_add_char.call_deferred(pipe.get_8())


func _add_char(c):
	# do something with the char


func clean_func():
	# close pipe and cleanup.
	pipe.close()
	thread.wait_to_finish()

@RoyalXXX
Copy link
Author

RoyalXXX commented Feb 2, 2025

In any case, it would be nice if Godot provided a way to get the number of bytes available for reading before actually reading. For example, in WinAPI, there is the PeekNamedPipe function, where the lpTotalBytesAvail parameter shows the number of bytes available for reading. In Unity C#, it is also possible to check before reading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants