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

Fix vte spawn_sync runtime check failed #1974

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

mlouielu
Copy link
Collaborator

When running Guake, spawn_sync shows runtime check failed:

$ guake
...
(guake:193534): VTE-WARNING **: 10:57:38.556: (../vte/src/vtepty.cc:667):bool
_vte_pty_spawn_sync(VtePty*, const char*, const char* const*,
                    const char* const*, GSpawnFlags, GSpawnChildSetupFunc,
                    gpointer, GDestroyNotify, GPid*, int,
                    GCancellable*, GError**):
runtime check failed: ((spawn_flags & ignored_spawn_flags()) == 0)
...

This is cause by setting DO_NOT_REAP_CHILD flag in it.

As shown in vte document & source code, DO_NOT_REAP_CHILD is
the default behavior for spawn_sync, so we don't need to pass
it when we try to spawn_sync.

When running Guake, `spawn_sync` shows runtime check failed:

```
$ guake
...
(guake:193534): VTE-WARNING **: 10:57:38.556: (../vte/src/vtepty.cc:667):bool
_vte_pty_spawn_sync(VtePty*, const char*, const char* const*,
                    const char* const*, GSpawnFlags, GSpawnChildSetupFunc,
                    gpointer, GDestroyNotify, GPid*, int,
                    GCancellable*, GError**):
runtime check failed: ((spawn_flags & ignored_spawn_flags()) == 0)
...
```

This is cause by setting `DO_NOT_REAP_CHILD` flag in it.

As shown in vte document & source code, `DO_NOT_REAP_CHILD` is
the default behavior for `spawn_sync`, so we don't need to pass
it when we try to `spawn_sync`.
@mlouielu mlouielu added this to the 3.8.2 milestone Nov 16, 2021
@mlouielu mlouielu requested a review from Davidy22 November 16, 2021 03:06
@mlouielu mlouielu self-assigned this Nov 16, 2021
Copy link
Collaborator

@Davidy22 Davidy22 left a comment

Choose a reason for hiding this comment

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

Looks good, code runs but I actually already knew that from when I was adding the SPAWN_NO_PARENT_ENVV flag, I left the flag there out of worry of breaking something that I was unaware of but I guess I didn't have to worry.

In somewhat related matters, I tried to replace spawn_sync with spawn_async but gave up partway because it broke an ungodly amount of things. spawn_sync is deprecated though, and eventually we probably want to make the move

@Davidy22 Davidy22 merged commit 5de0549 into master Nov 16, 2021
@mlouielu
Copy link
Collaborator Author

Lets add migrate to spawn_async into backlog?

@mlouielu mlouielu deleted the fix-vte-spawn-sync-runtime-check-failed branch November 16, 2021 03:17
@Davidy22
Copy link
Collaborator

It was brought up as an aside in #1950, and I think it's probably the fix for that and some other issues too. I also noticed it incidentally resolved #1826 too when I was testing it

PhungXuanAnh pushed a commit to PhungXuanAnh/guake that referenced this pull request Oct 28, 2023
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.

2 participants