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.is_process_running() returns false for a process created with OS.create_instance() #94974

Closed
passivestar opened this issue Jul 31, 2024 · 9 comments · Fixed by #94978
Closed

Comments

@passivestar
Copy link
Contributor

Tested versions

v4.3.rc1.official [e343dbb]

System information

Godot v4.3.rc1 - macOS 14.5.0 - Vulkan (Forward+) - integrated Apple M1 Max - Apple M1 Max (10 Threads)

Issue description

When creating a new godot process with OS.create_instance() and trying to monitor its lifecycle with OS.is_process_running() the latter method always return false. I'm trying to find a way to wait for the child process to finish before executing more logic code, but since OS.is_process_running() is always returning false it seems impossible

This is the code that the MRP is running:

@tool
extends Node

func _ready():
	var path := "res://child_project/"
	var absolute_path := ProjectSettings.globalize_path(path)

	var pid := OS.create_instance(["-e", "--path", absolute_path])
	print(OS.is_process_running(pid))

	await get_tree().process_frame
	print(OS.is_process_running(pid))

	await get_tree().create_timer(1.0).timeout
	print(OS.is_process_running(pid))

	await get_tree().create_timer(5.0).timeout
	print(OS.is_process_running(pid))

And this is the output:

Screenshot

The child project itself runs fine. The editor window with the correct project opens with no problem

Steps to reproduce

  1. Open the MRP project
  2. It will immediately open a new editor window with another project
  3. Close the editor with that project and go to the first window
  4. Look at the output. It prints false 4 times

Minimal reproduction project (MRP)

MRP_is_process_running.zip

@AThousandShips
Copy link
Member

AThousandShips commented Jul 31, 2024

Does it work correctly with create_process? The description of is_process_running specifically says it:

pid must be a valid ID generated from create_process.

So either this documentation description is wrong (and something is broken) or the description is correct (and we might want to clarify this further)

Also is this running from an app bundle? From a look at the macOS code it does its own handling for process creation in that case and doesn't seem to register running processes like normal in OSUnix

@akien-mga
Copy link
Member

akien-mga commented Jul 31, 2024

Does it work correctly with create_process? The description of is_process_running specifically says it:

pid must be a valid ID generated from create_process.

So either this documentation description is wrong (and something is broken) or the description is correct (and we might want to clarify this further)

create_instance is a wrapper for create_process, so the documentation is correct, but exposing an implementation detail.

Edit: Only in the base OS implementation. In the macOS one, it can redirect to create_process, but there's also an if (bundle) where it uses its own ad hoc method.

@passivestar
Copy link
Contributor Author

@AThousandShips

Also is this running from an app bundle?

Yes, this is rc1 bundle from the website

With this substitution it works correctly, prints true all 4 times:

var pid := OS.create_process(OS.get_executable_path(), ["-e", "--path", absolute_path])

However, docs say that OS.get_executable_path() isn't cross-platform, which is why I went for create_instance():

image

I wonder if it means that it's somehow not guaranteed to work in some cases. Git blame points to this commit by @AdamLearns: d7f4b07

@passivestar passivestar changed the title OS.is_process_running() returns false for a newly created godot instance OS.is_process_running() returns false for a process created with OS.create_instance() Jul 31, 2024
@akien-mga
Copy link
Member

CC @bruvzg

@bruvzg
Copy link
Member

bruvzg commented Jul 31, 2024

Seems like Unix is_process_running was changed to use PID map (in #90358), but it's not accounted when launching using macOS specific code.

@bruvzg
Copy link
Member

bruvzg commented Jul 31, 2024

I wonder if it means that it's somehow not guaranteed to work in some cases. Git blame points to this commit by

You can use OS.get_executable_path() on macOS, but launching the executable from a bundled GUI app directly can have some negative effects, e.g., main window can be unfocused on start, and app missing permissions, so it's not recommended.

@akien-mga akien-mga added this to the 4.3 milestone Jul 31, 2024
@passivestar
Copy link
Contributor Author

You can use OS.get_executable_path() on macOS, but launching the executable from a bundled GUI app directly can have some negative effects, e.g., main window can be unfocused on start, and app missing permissions, so it's not recommended

I see. I would actually prefer to run it headlessly because I just want to parse some scripts in the child process and quit, so opening the editor window at all isn't ideal for what I'm trying to do. But unfortunately --headless doesn't seem to work with the editor flag, it just gets ignored. So as far as I can tell there's no way to run code headlessly with access to editor APIs. It could be useful for all sorts of automation tasks

@akien-mga
Copy link
Member

--headless was broken by a regression in 4.3.rc1, but it should work in master or beta3.

@passivestar
Copy link
Contributor Author

--headless was broken by a regression in 4.3.rc1, but it should work in master or beta3.

ooh good to know, will test it out later 👍

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.

4 participants