The current code is unsafe to use from multiple threads at once.
GIOStream functions like this are supposed to be semi-threadsafe. It's
allowed for them to be called on both a reader thread and a writer
thread at the same time. Of course, it's still tricky and dangerous,
because it's only *really* threadsafe if the handshake has finished,
and API users have no plausible way to know that because the API
does not require performing an explicit handshake operation. But that's
a glib-networking problem. We can at least avoid the most obvious
threadsafety issue here in the API layer.
Note that we'll need to implement the new vfunc in glib-networking for
this to actually work.
Fixes#2393
The documentation for `g_bus_watch_name()` implies that the
`GDestroyNotify` for the user data will be called in the current thread
default `GMainContext`. Currently, it could be called in any thread, as
`client_unref()` can be called in any thread.
Fix that by deferring it to an idle source if `client_unref()` finalises
the `Client` object in a different thread.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
These were missing from the test before the previous commit ported from
`GMainLoop` to `GMainContext`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
It makes combination exit conditions a lot easier than when using
`g_main_loop_quit()` from different callbacks.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The first sentence incorrectly said that it checked the type of the
value, and then the second sentence explicitly said it was a programmer
error to give a value of the wrong type.
According to the code, the second sentence is correct.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2425
WebKit wants these private key properties to be readable in order to
implement a deserialization function. Currently they are read-only
because at the time GTlsCertificate was originally designed, the plan
was to support PKCS#11-backed private keys: private keys that are stored
on a smartcard, where the private key is completely unreadable. The
design goal was to support both memory-backed and smartcard-backed
private keys with the same GTlsCertificate API, abstracting away the
implementation differences such that code using GTlsCertificate doesn't
need to know the difference.
The original PKCS#11 implementation was never fully baked and at some
point in the past I deleted it all. It has since been replaced with a
new implementation, including a GTlsCertificate:private-key-pkcs11-uri
property, which is readable. So our current API already exposes the
differences between normal private keys and PKCS#11-backed private keys.
The point of making the private-key and private-key-pem properties
write-only was to avoid exposing this difference.
Do we have to make this API function readable? No, because WebKit could
be just as well served if we were to expose serialize and deserialize
functions instead. But WebKit needs to support serializing and
deserializing the non-private portion of GTlsCertificate with older
versions of GLib anyway, so we can do whatever is nicest for GLib. And I
think making this property readable is nicest, since the original design
reason for it to not be readable is now obsolete. The disadvantage to
this approach is that it's now possible for an application to read the
private-key or private-key-pem property, receive NULL, and think "this
certificate must not have a private key," which would be incorrect if
the private-key-pkcs11-uri property is set. That seems like a minor
risk, but it should be documented.
On Unix platforms, wait() and friends yield an integer that encodes
how the process exited. Confusingly, this is usually not the same as
the integer passed to exit() or returned from main(): conceptually it's
an integer encoding of this tagged union:
enum { EXITED, SIGNALLED, ... } tag;
union {
int exit_status; /* if EXITED */
struct {
int terminating_signal;
bool core_dumped;
} terminating_signal; /* if SIGNALLED */
...
} detail;
Meanwhile, on Windows, wait statuses and exit statuses are
interchangeable.
I find that it's clearer what is going on if we are consistent about
referring to the result of wait() as a "wait status", and the value
passed to exit() as an "exit status".
GSubprocess already gets this right: g_subprocess_get_status() returns
the wait status, while g_subprocess_get_exit_status() genuinely returns
the exit status. However, the GSpawn family of APIs has tended to
conflate the two.
Confusingly, g_spawn_check_exit_status() has always checked a wait
status, and it would not be correct to pass an exit status to it; so
let's deprecate it in favour of g_spawn_check_wait_status(), which
does the same thing that g_spawn_check_exit_status() always did.
Code that needs backwards-compatibility with older GLib can use:
#if !GLIB_CHECK_VERSION(2, 69, 0)
#define g_spawn_check_wait_status(x) (g_spawn_check_exit_status (x))
#endif
Signed-off-by: Simon McVittie <smcv@collabora.com>
Confusingly, g_spawn_check_exit_status() takes a wait status, not an
exit status, so passing g_subprocess_get_exit_status() to it is
incorrect (although both encodings happen to use 0 to encode success
and a nonzero value to encode failure, so in practice this probably
had the desired effect).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Following on from the previous commit, some explicit
`g_main_context_wakeup()` calls were missing from the test code which
only uses `GMainContext`.
Add them, and also add some assertions to check that these functions are
being called in the expected thread (as the code comments say).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The tests in `gdbus-names.c` use a mixture of `GMainLoop` and iterating
a `GMainContext` directly. Some of the helper functions based around the
`OwnNameData` struct use the `loop` `GMainLoop` even when called from
tests like `watch_with_different_context()` which themselves use
`GMainContext` directly.
Thus, it’s possible for the `GMainLoop` to not be running, while the
test is iterating on `g_main_context_iteration()`. In this case,
`g_main_loop_quit()` is a no-op and will not wake up the `GMainContext`.
This causes the test to livelock in around 1 in 1200 test runs.
Fix this by adding an explicit `g_main_context_wakeup()` call after each
`g_main_loop_quit()` call. A more comprehensive fix would be to port all
the tests in this file to iterating `GMainContext` directly, and drop
all the `GMainLoop` usage, but I don’t have time for that right now.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If `g_file_monitor_source_dispatch()` drops the last reference to its
`GLocalFileMonitor`, a deadlock will occur, because disposing the
`GLocalFileMonitor` causes synchronous disposal of the
`GFileMonitorSource`, and hence an attempt to re-lock the already-locked
mutex in the `GFileMonitorSource`.
Fix that by dropping the reference to the `GLocalFileMonitor` after
unlocking.
Diagnosed by Ting-Wei Lan. The bug was originally introduced by me in
commit 592a13b483.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Relax the requirement for the test to only be compiled/run under gcc,
since a version of LLVM was released which supports `--add-symbol`.
`objcopy` should be overrideable to be `llvm-objcopy` by using a machine
file as per https://mesonbuild.com/Machine-files.html#binaries.
Suggested and tested by Grigory Vasilyev.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2423
Clarify that the terms ‘GUID’ and ‘UUID’ are used interchangeably in the
context of D-Bus, and that neither of them are an RFC 4122 UUID.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Update several links to allow the remote to use its configured default
branch name, rather than specifying `master` as the default branch name.
This will help avoid breakage if any of these projects rename their
default branch in the future.
Fix a few of the links where they were hitting redirects or had moved.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2348
Since commit 87e19535fe, the ETag check when writing out a file through
a symlink (following the symlink) has been incorrectly using the ETag
value of the symlink, rather than the target file. This is incorrect
because the ETag should represent the file content, not its metadata or
links to it.
Fix that, and add a unit test.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2417
This adds g_tls_connection_get_protocol_version(),
g_tls_connection_get_ciphersuite_name(), and DTLS variants. This will
allow populating TLS connection information in the WebKit web inspector.
This is WIP because we found it's not quite possibly to implement
correctly with GnuTLS. See glib-networking!151.
This is the result of checking each `Returns:` line in these files. I’ve
only considered nullability and not other (potentially missing or
incorrect) annotations.
Including suggestions by Simon McVittie.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
In a PKCS#11 operation there are multiple types of PINs possibly
needed and these flags add a way to expose them to the user.
This design exactly matches gnutls' gnutls_pin_flag_t API.
This changeset exposes
* `not-valid-before`
* `not-valid-after`
* `subject-name`
* `issuer-name`
on GTlsCertificate provided by the underlying TLS Backend.
In order to make use of these changes,
see the related [glib-networking MR][glib-networking].
This change aims to help populate more of the [`Certificate`][wk-cert]
info in the WebKit Inspector Protocol on Linux.
This changeset stems from work in Microsoft Playwright to [add more info
into its HAR capture][pw] generated from the Inspector Protocol events
and will bring feature parity across WebKit platforms.
[wk-cert]: 8afe31a018/Source/JavaScriptCore/inspector/protocol/Security.json
[pw]: https://github.com/microsoft/playwright/pull/6631
[glib-networking]: https://gitlab.gnome.org/GNOME/glib-networking/-/merge_requests/156
The `GApplication` must be registered before calling
`g_application_mark_busy()`. Document that, and add a guard.
The same is true for `g_application_unmark_busy()`, but the existing
documentation and guard for `busy_count > 0` are enough.
For the reasons given in the new bit of documentation, GDBusProxy should
not be used for connecting to stateless D-Bus services which may be
restarted at any point.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1036
Include the base URI in the `g_test_bug()` calls instead. This resolves
inconsistencies between the old bug base (bugzilla.gnome.org) and the
new bug base (gitlab.gnome.org). It also has the advantage that the URI
passed to `g_test_bug()` is now clickable in the code editor, rather
than being split across two locations.
See https://gitlab.gnome.org/GNOME/glib/-/merge_requests/275#note_303175
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Since commit ab285899a6 ('gdbusconnection: Document main context
iteration for unsubscript'), we document when the user is guaranteed
that all resources are gone after g_dbus_connection_signal_unsubscribe().
This is not merely an implementation detail, it's something that the
user needs to be able to rely on. It is good that this is documented.
However, libnm does something different ([1]). It registers to several D-Bus
signals without providing a GDestroyNotify. After unsubscription, it schedules
another idle action with lower priority and uses that to know when
cleanup is complete. I think this is a useful alternative and should
also be guaranteed and documented to work.
Also note that this isn't just some implementation detail that currently
happens to work. GDBusConnection tightly integrates with GMainContext and it
works by scheduling idle sources with G_PRIORITY_DEFAULT priority. It needs to
schedule all events with this same priority, otherwise the ordering is not
preserved. At this point, with GDBusConnection working this way, this is no longer
something that can reasonably be any different. It's how GDBusConnection fundamentally
works, and a user must be able to rely on that. As such, this new promise isn't
something that we would want to break in the future.
Thus document it.
[1] a55c10c6cb/src/libnm-client-impl/nm-client.c (L7918)
This fixes a bug where the family flag was ignored in lookup_data_new,
causing the resolver to call getaddrinfo with no hints set when clearly
the family hint should have been set.
gio/tests/unix-streams.c: In function ‘test_write_async_wouldblock’:
gio/tests/unix-streams.c:692:17: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘gint’ {aka ‘int’}
692 | for (i = 0; i < 4 * pipe_capacity; i++)
| ^
gio/tests/unix-streams.c: In function ‘test_writev_async_wouldblock’:
gio/tests/unix-streams.c:780:17: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘gint’ {aka ‘int’}
780 | for (i = 0; i < 4 * pipe_capacity; i++)
| ^
An application that has been shut down is still marked as registered
even if its implementation has been already destroyed.
This may lead to unguarded crashes when calling functions that have
assumptions for being used with registered applications.
So, when an application is registered, mark it as unregistered just
before destroying its implementation and after being shut down, so that
we follow the registration process in reversed order.
Added tests
gio/tests/socket-common.c: In function ‘socket_address_from_string’:
gio/tests/socket-common.c:50:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’
50 | for (i = 0; i < G_N_ELEMENTS (unix_socket_address_types); i++)
| ^
gio/tests/gnotification-server.c: In function ‘g_notification_server_bus_acquired’:
gio/tests/gnotification-server.c:224:3: error: missing initializer for field ‘padding’ of ‘GDBusInterfaceVTable’ {aka ‘const struct _GDBusInterfaceVTable’}
224 | };
| ^
gio/tests/gdbus-proxy.c: In function ‘strv_equal’:
gio/tests/gdbus-proxy.c:158:32: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘gint’ {aka ‘int’}
158 | res = g_strv_length (strv) == count;
| ^~
gio/gkeyfilesettingsbackend.c: In function ‘convert_path’:
gio/gkeyfilesettingsbackend.c:155:15: warning: comparison of integer
expressions of different signedness: ‘gsize’ {aka ‘long unsigned int’}
and ‘gint’ {aka ‘int’}
155 | if (key_len < kfsb->prefix_len ||
| ^
