Commit Graph

30 Commits

Author SHA1 Message Date
Sergio Costas
c12762a091 GSubprocessLauncher: allow to close passed FDs
By default, when using g_subprocess_launcher_take_fd() to pass an
FD to a child, the GSubprocessLauncher object also takes ownership
of the FD in the parent, and closes it during finalize(). This is
a reasonable assumption in the majority of the cases, but sometimes
it isn't a good idea.

An example is when creating a GSubprocessLauncher in JavaScript:
here, the destruction process is managed by the Garbage Collector,
which means that those sockets will remain opened for some time
after all the references to the object has been droped. This means
that it could be not possible to detect when the child has closed
that same FD, because in order to make that work, both FDs
instances (the one in the parent and the one in the children) must
be closed. This can be a problem in, as an example, a process that
launches a child that communicates with Wayland using an specific
socket (like when using the new API MetaWaylandClient).

Of course, it isn't a valid solution to manually call close() in
the parent process just after the call to spawn(), because the FD
number could be reused in the time between it is manually closed,
and when the object is destroyed and closes again that FD. If that
happens, it will close an incorrect FD.

One solution could be to call run_dispose() from Javascript on the
GSubprocessLauncher object, to force freeing the resources.
Unfortunately, the current code frees them in the finalize()
method, not in dispose() (this is fixed in !1670 (merged) ) but it
isn't a very elegant solution.

This proposal adds a new method, g_subprocess_launcher_close(),
that allows to close the FDs passed to the child. To avoid problems,
after closing an FD with this method, no more spawns are allowed.

Fix: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1677
2020-10-12 20:29:48 +02:00
Philip Withnall
00bfb3ab44 tree: Fix various typos and outdated terminology
This was mostly machine generated with the following command:
```
codespell \
    --builtin clear,rare,usage \
    --skip './po/*' --skip './.git/*' --skip './NEWS*' \
    --write-changes .
```
using the latest git version of `codespell` as per [these
instructions](https://github.com/codespell-project/codespell#user-content-updating).

Then I manually checked each change using `git add -p`, made a few
manual fixups and dropped a load of incorrect changes.

There are still some outdated or loaded terms used in GLib, mostly to do
with git branch terminology. They will need to be changed later as part
of a wider migration of git terminology.

If I’ve missed anything, please file an issue!

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-06-12 15:01:08 +01:00
Philip Withnall
7f256e63d0 tests: Fix small race in GSubprocess tests
This was introduced in commit 7846d6154a: g_subprocess_get_identifier()
will return NULL after the subprocess has exited, and the subprocess in
the `noop` test will exit as soon as it has started spawning. So if the
scheduler scheduled the testprog subprocess quickly, descheduled the
parent test process until the testprog exited, then the return value
from g_subprocess_get_identifier() would be NULL.

Move the g_subprocess_get_identifier() test to one which calls testprog
in `sleep-forever` mode, since that is guaranteed not to exit until
killed (which we do later in the test).

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-05-31 23:35:27 +01:00
Philip Withnall
277c42ddc0 tests: Convert g_assert() to g_assert_*() in gio/tests/gsubprocess.c
g_assert_*() give more informative failure messages, and aren’t compiled
out when building with G_DISABLE_ASSERT.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-03-08 19:46:20 +00:00
Руслан Ижбулатов
1cc78bf082 GSubprocess test: Adjust the cwd test to pass on Windows
There's no /tmp directory on Windows.
Use g_get_tmp_dir(), and adjust the test to work with that.
The test *still* checks the basename of the new CWD, it just
doesn't need to be "tmp" anymore.
2019-02-13 08:52:59 +00:00
Руслан Ижбулатов
4bc1aa23a9 GSubprocess test: Add PATH to the envtable that is passed to child
envp in spawn() functions is the *whole* environment table
for the child process. Including PATH. Thus, unless PATH is explicitly
put into that table, the process will be spawned without PATH.
Since on Windows binaries are found via PATH instead of LD_LIBRARY_PATH
or whatever, almost no program (unless installed in WINDIR, maybe)
can run without a PATH. Certainly not test programs - meson
adds bld subdirs to the PATH to make sure that test programs
use uninstalled glib at runtime.
So make sure that PATH is passed along.
2019-02-13 08:52:58 +00:00
Руслан Ижбулатов
01642d6db5 GSubprocess test: Use LINEEND more
Windows \r\n EOLs strike again. The test already knows about LINEEND,
so make it use LINEEND more (instead of swithcing pipes
to binary mode). This also applies to counting the bytes
read.
2019-02-13 08:52:58 +00:00
Philip Withnall
19c7a7bb23 gsubprocess: Add a missing test for invalid UTF-8 output
There were tests for invalid UTF-8 output when asynchronously
communicating with a subprocess, but nothing for synchronous
communication. Add such a test, and refine the code as a result.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-10-11 11:31:27 +13:00
Marco Trevisan (Treviño)
72bf795db5 tests, subprocess: units for cancelled communications 2018-08-27 17:08:47 +02:00
Xavier Claessens
5ccd9449fb Tests: gsubprocess: Do not use stdin/stdout variables
https://bugzilla.gnome.org/show_bug.cgi?id=795406
2018-04-20 15:33:31 -04:00
Philip Withnall
66ab836f5a gsubprocess: Fix a critical calling communicate() with no pipes
If calling g_subprocess_communicate() on a GSubprocess with no
stdout/stderr pipe, a critical warning would be emitted from
g_memory_output_stream_steal_as_bytes(), as it would be called on a NULL
output stream.

Fix that, improve the relevant GIR annotations, and expand the unit
tests to cover it (and various other combinations of flags).

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=793331
2018-02-13 16:27:32 +00:00
Philip Withnall
73eee8d64e tests: Fix some leaks and double-frees in the GSubprocess tests
Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=786456
2017-08-23 11:37:32 +01:00
Will Thompson
65a95a5a2d gio: failing cases for subprocess cancellable bug
https://bugzilla.gnome.org/show_bug.cgi?id=786456
2017-08-23 11:37:32 +01:00
Philip Withnall
e1e73dafa6 gsubprocess: Copy parent process’ environ when clearing subprocess’
Previously, this was done at the time of spawning the subprocess, which
meant the g_subprocess_launcher_*_environ() functions could not be used
to modify the parent process’ environment.

Change the code to copy the parent process’ environment when
g_subprocess_launcher_set_environ(NULL) is called. Document the change
and add a unit test.

https://bugzilla.gnome.org/show_bug.cgi?id=778422
2017-05-31 12:07:31 +01:00
Dan Winship
9f2e3f6b72 gtestutils: add g_assert_cmpmem()
Add a test macro to compare two buffers (which are not already known
to be the same length) for equality.

https://bugzilla.gnome.org/show_bug.cgi?id=754283
2015-08-31 13:59:48 -04:00
Ryan Lortie
2b11af4f1f gsubprocess: test environment a bit more
Add a test for GSubprocess to test setting, unsetting and inheritance of
environment variables.  Use communicate() to give it a bit more of a
workout as well.

https://bugzilla.gnome.org/show_bug.cgi?id=725651
2014-03-04 09:08:24 -05:00
Ryan Lortie
9da88a12c5 gsubprocess: test empty splices
Make sure we handle the case that our splice returns no data properly.

https://bugzilla.gnome.org/show_bug.cgi?id=724916
2014-03-03 21:26:24 -05:00
Matthias Clasen
7846d6154a Improve GSubprocess test coverage
Test the synchronous communicate functions.
2014-01-01 17:59:20 -05:00
Colin Walters
7a90c201f7 tests/gsubprocess: Only compare basename of /tmp
Otherwise, we break in Continuous where /tmp == /sysroot/tmp,
and something in the execve() or the kernel will do realpath()
for us.
2013-11-29 04:12:26 +00:00
Matthias Clasen
a688b2a9c0 Improve test coverage for GSubprocess 2013-11-28 16:01:23 -05:00
Matthias Clasen
7d82d6dc5b Test g_subprocess_launcher_spawn
The spawnv variant of this function already had a test, but
spawn was not tested.
2013-11-28 01:36:29 -05:00
Matthias Clasen
4a687ed723 Test some more GSubProcess api
This covers g_subprocess_set_environ, g_subprocess_setenv,
g_subprocess_getenv, and g_subprocess_set_flags.
2013-11-28 01:12:26 -05:00
Philip Withnall
c9ccc2af91 gio/tests: Add non-NULL assertions to help static analysis
These prevent some false positives from the static analyser which are
caused by it not inspecting the invariants of
g_subprocess_communicate[_utf8]_finish() (i.e. that stdout and
stdout_str will always be set unless an error was returned).

They’re also good testing anyway.

Found by scan-build.

https://bugzilla.gnome.org/show_bug.cgi?id=113075
2013-11-27 10:01:29 +00:00
Chun-wei Fan
b27a2d436b gio/tests/gsubprocess.c: Fix on Windows
We need to check for the correct line endings on Windows (\r\n) for the
echo tests and currently need to skip the test_echo_eof test there, as
it depends on the cat utility that is not normally found on Windows, and
using an external installation of cat via MSYS or Cygwin would render the
test program to hang as cat waits for user input.

https://bugzilla.gnome.org/show_bug.cgi?id=711047
2013-11-11 22:38:05 +08:00
Stef Walter
95526b5db5 gsubprocess: Fix leaks in tests
https://bugzilla.gnome.org/show_bug.cgi?id=711802
2013-11-11 07:12:18 +01:00
Stef Walter
fe3c878c53 gsubprocess: Fix error detection logic in tests
Various tests were depending on local_error being set by a callback
when it could never have been the case. Simplify async error detection
logic in those cases, and fix leak of GError.

https://bugzilla.gnome.org/show_bug.cgi?id=711802
2013-11-11 07:12:00 +01:00
Colin Walters
ebd098155b tests/gsubprocess: Locate test child binary for installed tests too
Just use the new g_test_build_filename() rather than assuming
it's in cwd.
2013-10-17 22:39:48 +01:00
Colin Walters
9318d5a429 gsubprocess: Add UTF-8 variants of communicate()
Over many years of writing code interacting with subprocesses, a pattern
that comes up a lot is to run a child and get its output as UTF-8, to
put inside a JSON document or render in a GtkTextBuffer, etc.

It's very important to validate at the boundaries, and not say deep
inside Pango.

We could do this a bit more efficiently if done in a streaming fashion,
but realistically this should be OK for now.
2013-10-17 14:32:44 -04:00
Colin Walters
0e1a3ee345 gsubprocess: Fix up communicate
We weren't closing the streams after we were done reading or writing,
which is kind of essential.  The easy way to fix this is to just use
g_output_stream_splice() to a GMemoryOutputStream rather than
hand-rolling it.  This results in a substantial reduction of code
complexity.

A second serious issue is that we were marking the task as complete when
the process exits, but that's racy - there could still be data to read
from stdout.  Fix this by just refcounting outstanding operations.

This code, not surprisingly, looks a lot like the "multi" test.

Next, because processes output binary data, I'd be forced to annotate
the char*/length pairs as (array) (element-type uint8).  But rather than
doing that, it's *far* simpler to just use GBytes.

We need a version of this that actually validates as UTF-8, that will be
in the next patch.
2013-10-17 14:32:44 -04:00
Colin Walters
5b48dc40cc GSubprocess: New class for spawning child processes
There are a number of nice things this class brings:

0) Has a race-free termination API on all platforms (on UNIX, calls to
   kill() and waitpid() are coordinated as not to cause problems).
1) Operates in terms of G{Input,Output}Stream, not file descriptors
2) Standard GIO-style async API for wait() with cancellation
3) Makes some simple cases easy, like synchronously spawning a
   process with an argument list
4) Makes hard cases possible, like asynchronously running a process
   with stdout/stderr merged, output directly to a file path

Much rewriting and code review from Ryan Lortie <desrt@desrt.ca>

https://bugzilla.gnome.org/show_bug.cgi?id=672102
2013-10-17 14:32:44 -04:00