From af560fc1da26a7d0e94b0d56a3cd2a14d2b082c8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 16:41:42 +0200 Subject: [PATCH 1/6] update TODO --- TODO | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/TODO b/TODO index 3ca3c5ec35a..9d3a4d13b61 100644 --- a/TODO +++ b/TODO @@ -23,6 +23,13 @@ Janitorial Clean-ups: Features: +* optionally, if a per-partition GPT flag is set for the root/home/… partitions + format the partition on next boot and unset the flag, in order to implement + factory reset. also, add a second flag that simply indicates whether such a + scheme is supported. then, add a tool (or maybe beef up systemd-dissect) to + show state of these flags, and optionally trigger such a factory reset on + next boot by setting the flag. + * logind: maybe watch utmp asynchronously using inotify, and populate our own tracked session metadata from the fields available therein. Why bother? Right now, all "ssh" sessions will be tracked without their TTY by logind (which is From 64a7ef8bc06b5dcfcd9f99ea10a43bde75c4370f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 16:43:30 +0200 Subject: [PATCH 2/6] man: be more explicit about thread safety of sd_journal Triggered by https://bugzilla.redhat.com/show_bug.cgi?id=1609349 This adds two generic paragaphs we include via xinclude. One is the "strict" version, which contains wording saying that we are thread agnostic and what that means. And the other is the "safe" version, for the cases we provide fully safety. Let's then change most man pages to use either of these generic paragraphs. With one exception: man/sd_journal_get_catalog.xml contains both kinds of function, we hence use manual wording. --- man/libudev.xml | 8 +++----- man/sd-journal.xml | 17 ++++++++--------- man/sd_journal_enumerate_fields.xml | 3 +-- man/sd_journal_get_catalog.xml | 11 ++++++++--- man/sd_journal_get_cursor.xml | 3 +-- man/sd_journal_get_cutoff_realtime_usec.xml | 3 +-- man/sd_journal_get_data.xml | 8 +++++++- man/sd_journal_get_fd.xml | 11 +++-------- man/sd_journal_get_realtime_usec.xml | 8 +++++++- man/sd_journal_get_usage.xml | 3 +-- man/sd_journal_has_runtime_files.xml | 3 +-- man/sd_journal_next.xml | 3 +-- man/sd_journal_open.xml | 13 ++++--------- man/sd_journal_print.xml | 3 ++- man/sd_journal_query_unique.xml | 3 +-- man/sd_journal_seek_head.xml | 3 +-- man/sd_journal_stream_fd.xml | 3 +-- man/threads-aware.xml | 17 +++++++++++++++++ 18 files changed, 68 insertions(+), 55 deletions(-) create mode 100644 man/threads-aware.xml diff --git a/man/libudev.xml b/man/libudev.xml index 8cb4ba59fc3..382c1aa25c5 100644 --- a/man/libudev.xml +++ b/man/libudev.xml @@ -48,11 +48,9 @@ udev_new3. It is used to track library state and link objects together. No global state is used by libudev, everything is always linked to - a udev context. Furthermore, multiple different udev contexts can - be used in parallel by multiple threads. However, a single context - must not be accessed by multiple threads in parallel. The caller - is responsible for providing suitable locking if they intend to use - it from multiple threads. + a udev context. + + To introspect a local device on a system, a udev device object can be created via diff --git a/man/sd-journal.xml b/man/sd-journal.xml index 8bfcb90ca0e..3fa6c75b7e9 100644 --- a/man/sd-journal.xml +++ b/man/sd-journal.xml @@ -77,16 +77,15 @@ Thread safety - Functions that operate on the sd_journal object are thread - agnostic — given sd_journal pointer may only be used from one thread at - a time, but multiple threads may use multiple such objects safely. Other functions — - those that are used to send entries to the journal, like - sd_journal_print3 - and similar, or those that are used to retrieve global information like - sd_journal_stream_fd3 - and + Functions that operate on sd_journal objects are thread agnostic — given + sd_journal pointer may only be used from one specific thread at all times (and it has to + be the very same one during the entire lifetime of the object), but multiple, independent threads may use multiple, + independent objects safely. Other functions — those that are used to send entries to the journal, like + sd_journal_print3 and similar, + or those that are used to retrieve global information like + sd_journal_stream_fd3 and sd_journal_get_catalog_for_message_id3 - — are thread-safe and may be called from multiple threads in parallel. + — are fully thread-safe and may be called from multiple threads in parallel. diff --git a/man/sd_journal_enumerate_fields.xml b/man/sd_journal_enumerate_fields.xml index 95af2c1ee0a..c5704f53adf 100644 --- a/man/sd_journal_enumerate_fields.xml +++ b/man/sd_journal_enumerate_fields.xml @@ -86,8 +86,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_get_catalog.xml b/man/sd_journal_get_catalog.xml index ce37e177bd1..80edc08c812 100644 --- a/man/sd_journal_get_catalog.xml +++ b/man/sd_journal_get_catalog.xml @@ -87,9 +87,14 @@ Notes - Function sd_journal_get_catalog() is thread-agnostic and only a - single thread may operate on a given sd_journal object. Function - sd_journal_get_catalog_for_message_id() is thread-safe. + Function sd_journal_get_catalog() is thread-agnostic and only + a single specific thread may operate on a given object during its entire lifetime. It's safe to allocate multiple + independent objects and use each from a specific thread in parallel. However, it's not safe to allocate such an + object in one thread, and operate or free it from any other, even if locking is used to ensure these threads don't + operate on it at the very same time. + + Function sd_journal_get_catalog_for_message_id() is are thread-safe and may be called in + parallel from multiple threads. diff --git a/man/sd_journal_get_cursor.xml b/man/sd_journal_get_cursor.xml index 6817a3cd542..d5e465b81ac 100644 --- a/man/sd_journal_get_cursor.xml +++ b/man/sd_journal_get_cursor.xml @@ -98,8 +98,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_get_cutoff_realtime_usec.xml b/man/sd_journal_get_cutoff_realtime_usec.xml index dc8e32bf81b..b2a0634f7dd 100644 --- a/man/sd_journal_get_cutoff_realtime_usec.xml +++ b/man/sd_journal_get_cutoff_realtime_usec.xml @@ -96,8 +96,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_get_data.xml b/man/sd_journal_get_data.xml index 99f9500441b..464fd16ace1 100644 --- a/man/sd_journal_get_data.xml +++ b/man/sd_journal_get_data.xml @@ -156,7 +156,13 @@ success or a negative errno-style error code. - + + Notes + + + + + Examples diff --git a/man/sd_journal_get_fd.xml b/man/sd_journal_get_fd.xml index 7edbc4bc251..2186b685bfc 100644 --- a/man/sd_journal_get_fd.xml +++ b/man/sd_journal_get_fd.xml @@ -226,14 +226,9 @@ else { Notes - The sd_journal_get_fd(), - sd_journal_get_events(), - sd_journal_reliable_fd(), - sd_journal_process() and - sd_journal_wait() interfaces are available as - a shared library, which can be compiled and linked to with the - libsystemd pkg-config1 - file. + + + diff --git a/man/sd_journal_get_realtime_usec.xml b/man/sd_journal_get_realtime_usec.xml index 2030e8372db..e0f5c4d2e98 100644 --- a/man/sd_journal_get_realtime_usec.xml +++ b/man/sd_journal_get_realtime_usec.xml @@ -89,7 +89,13 @@ sd_journal_get_monotonic_usec(). - + + Notes + + + + + See Also diff --git a/man/sd_journal_get_usage.xml b/man/sd_journal_get_usage.xml index 358a62d0663..39f53dd5eb6 100644 --- a/man/sd_journal_get_usage.xml +++ b/man/sd_journal_get_usage.xml @@ -56,8 +56,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_has_runtime_files.xml b/man/sd_journal_has_runtime_files.xml index b7bbf224d43..44fdc8d1868 100644 --- a/man/sd_journal_has_runtime_files.xml +++ b/man/sd_journal_has_runtime_files.xml @@ -66,8 +66,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_next.xml b/man/sd_journal_next.xml index c0ca5a8a14c..9a27d1426e3 100644 --- a/man/sd_journal_next.xml +++ b/man/sd_journal_next.xml @@ -122,8 +122,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_open.xml b/man/sd_journal_open.xml index 9f600b223f1..cf787b7ea1c 100644 --- a/man/sd_journal_open.xml +++ b/man/sd_journal_open.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_journal_open @@ -184,15 +185,9 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + - The sd_journal_open(), - sd_journal_open_directory() and - sd_journal_close() interfaces are available - as a shared library, which can be compiled and linked to with the - libsystemd pkg-config1 - file. + diff --git a/man/sd_journal_print.xml b/man/sd_journal_print.xml index f8ff7ba0932..e18cf88bbcf 100644 --- a/man/sd_journal_print.xml +++ b/man/sd_journal_print.xml @@ -177,7 +177,8 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid( Thread safety - All functions listed here are thread-safe and may be called in parallel from multiple threads. + + sd_journal_sendv() is "async signal safe" in the meaning of signal-safety7. diff --git a/man/sd_journal_query_unique.xml b/man/sd_journal_query_unique.xml index c62f333eabb..9adafa11446 100644 --- a/man/sd_journal_query_unique.xml +++ b/man/sd_journal_query_unique.xml @@ -126,8 +126,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_seek_head.xml b/man/sd_journal_seek_head.xml index 86274071f53..da88d241e87 100644 --- a/man/sd_journal_seek_head.xml +++ b/man/sd_journal_seek_head.xml @@ -120,8 +120,7 @@ Notes - All functions listed here are thread-agnostic and only a single thread may operate - on a given sd_journal object. + diff --git a/man/sd_journal_stream_fd.xml b/man/sd_journal_stream_fd.xml index de76cabb4d4..8e65dc76580 100644 --- a/man/sd_journal_stream_fd.xml +++ b/man/sd_journal_stream_fd.xml @@ -92,8 +92,7 @@ Notes - Function sd_journal_stream_fd() is thread-safe and may be called - from multiple threads. + diff --git a/man/threads-aware.xml b/man/threads-aware.xml new file mode 100644 index 00000000000..7985f4acd19 --- /dev/null +++ b/man/threads-aware.xml @@ -0,0 +1,17 @@ + + + + + + + +All functions listed here are thread-agnostic and only a single specific thread may operate on a +given object during its entire lifetime. It's safe to allocate multiple independent objects and use each from a +specific thread in parallel. However, it's not safe to allocate such an object in one thread, and operate or free it +from any other, even if locking is used to ensure these threads don't operate on it at the very same time. + +All functions listed here are thread-safe and may be called in parallel from multiple threads. + + From b4f607433cac749b617e15b3d5d122322ed2bc71 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 16:45:21 +0200 Subject: [PATCH 3/6] hashmap: add an environment variable to turn off the memory pool used by hashmaps Triggered by https://bugzilla.redhat.com/show_bug.cgi?id=1609349 --- doc/ENVIRONMENT.md | 3 +++ src/basic/hashmap.c | 24 ++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/doc/ENVIRONMENT.md b/doc/ENVIRONMENT.md index c69bf9b6646..9d598a66931 100644 --- a/doc/ENVIRONMENT.md +++ b/doc/ENVIRONMENT.md @@ -42,6 +42,9 @@ All tools: are understood, too (us, ms, s, min, h, d, w, month, y). If it is not set or set to 0, then the built-in default is used. +* `$SYSTEMD_MEMPOOL=0` — if set the internal memory caching logic employed by + hash tables is turned off, and libc malloc() is used for all allocations. + systemctl: * `$SYSTEMCTL_FORCE_BUS=1` — if set, do not connect to PID1's private D-Bus diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 69a7d70b04b..65e22ccd21f 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -6,8 +6,9 @@ #include #include "alloc-util.h" -#include "hashmap.h" +#include "env-util.h" #include "fileio.h" +#include "hashmap.h" #include "macro.h" #include "mempool.h" #include "process-util.h" @@ -766,20 +767,31 @@ static void reset_direct_storage(HashmapBase *h) { memset(p, DIB_RAW_INIT, sizeof(dib_raw_t) * hi->n_direct_buckets); } +static bool use_pool(void) { + static int b = -1; + + if (!is_main_thread()) + return false; + + if (b < 0) + b = getenv_bool("SYSTEMD_MEMPOOL") != 0; + + return b; +} + static struct HashmapBase *hashmap_base_new(const struct hash_ops *hash_ops, enum HashmapType type HASHMAP_DEBUG_PARAMS) { HashmapBase *h; const struct hashmap_type_info *hi = &hashmap_type_info[type]; - bool use_pool; + bool up; - use_pool = is_main_thread(); - - h = use_pool ? mempool_alloc0_tile(hi->mempool) : malloc0(hi->head_size); + up = use_pool(); + h = up ? mempool_alloc0_tile(hi->mempool) : malloc0(hi->head_size); if (!h) return NULL; h->type = type; - h->from_pool = use_pool; + h->from_pool = up; h->hash_ops = hash_ops ? hash_ops : &trivial_hash_ops; if (type == HASHMAP_TYPE_ORDERED) { From 205c085bc36c2c61a09dc40621d8561b135d9b57 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 16:46:26 +0200 Subject: [PATCH 4/6] hashmap: add an explicit assert() for detecting when objects migrated between threads When clients don't follow protocol and use the same object from different threads, then we previously would silently corrupt memory. With this assert we'll fail with an assert(). This doesn't fix anything but certainly makes mis-uses easier to detect and debug. Triggered by https://bugzilla.redhat.com/show_bug.cgi?id=1609349 --- src/basic/hashmap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 65e22ccd21f..44d718c83d2 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -869,9 +869,11 @@ static void hashmap_free_no_clear(HashmapBase *h) { assert_se(pthread_mutex_unlock(&hashmap_debug_list_mutex) == 0); #endif - if (h->from_pool) + if (h->from_pool) { + /* Ensure that the object didn't get migrated between threads. */ + assert_se(is_main_thread()); mempool_free_tile(hashmap_type_info[h->type].mempool, h); - else + } else free(h); } From 7612719ef4d322bff9136e88518fee668c0c77e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 16:48:53 +0200 Subject: [PATCH 5/6] test-resolved: follow coding style, use fopen() with 'e' --- src/resolve/test-resolved-etc-hosts.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/test-resolved-etc-hosts.c b/src/resolve/test-resolved-etc-hosts.c index 5b8f1e220e3..b7dba0d7800 100644 --- a/src/resolve/test-resolved-etc-hosts.c +++ b/src/resolve/test-resolved-etc-hosts.c @@ -9,7 +9,7 @@ static void test_parse_etc_hosts_system(void) { _cleanup_fclose_ FILE *f = NULL; - f = fopen("/etc/hosts", "r"); + f = fopen("/etc/hosts", "re"); if (!f) { assert_se(errno == -ENOENT); return; @@ -27,7 +27,7 @@ static void test_parse_etc_hosts(const char *fname) { _cleanup_fclose_ FILE *f; if (fname) { - f = fopen(fname, "r"); + f = fopen(fname, "re"); assert_se(f); } else { fd = mkostemp_safe(t); From 7647c01d8f55d5ba1c918d103773252cb09ebf23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 17:34:40 +0200 Subject: [PATCH 6/6] man: include libsystemd-pkgconfig.xml in a few more man pages for some reason this was forgotten for a couple of sd_bus man pages, let's fix that. --- man/sd-bus-errors.xml | 13 +++---------- man/sd_bus_add_match.xml | 6 ++---- man/sd_bus_error_add_map.xml | 13 +++---------- man/sd_bus_is_open.xml | 11 +++-------- man/sd_bus_path_encode.xml | 12 ++---------- man/sd_bus_request_name.xml | 11 +++-------- man/sd_bus_set_connected_signal.xml | 11 +++-------- man/sd_bus_set_sender.xml | 11 +++-------- man/sd_bus_set_watch_bind.xml | 11 +++-------- 9 files changed, 25 insertions(+), 74 deletions(-) diff --git a/man/sd-bus-errors.xml b/man/sd-bus-errors.xml index 50abf4c076c..c896511541e 100644 --- a/man/sd-bus-errors.xml +++ b/man/sd-bus-errors.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd-bus-errors @@ -259,15 +260,7 @@ - - Notes - - The various error definitions described here are available - as a shared library, which can be compiled and linked to with the - libsystemd pkg-config1 - file. - + See Also diff --git a/man/sd_bus_add_match.xml b/man/sd_bus_add_match.xml index a22d443ce7a..c4f24aed3e4 100644 --- a/man/sd_bus_add_match.xml +++ b/man/sd_bus_add_match.xml @@ -8,7 +8,7 @@ Copyright © 2016 Julian Orth --> - + sd_bus_add_match @@ -154,9 +154,7 @@ Notes - sd_bus_add_match() and the other functions described here are available as a shared - library, which can be compiled and linked to with the libsystemd pkg-config1 file. + diff --git a/man/sd_bus_error_add_map.xml b/man/sd_bus_error_add_map.xml index 107291f26f7..dbe05a18926 100644 --- a/man/sd_bus_error_add_map.xml +++ b/man/sd_bus_error_add_map.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_error_add_map @@ -123,15 +124,7 @@ - - Notes - - The various error definitions described here are available - as a shared library, which can be compiled and linked to with the - libsystemd pkg-config1 - file. - + See Also diff --git a/man/sd_bus_is_open.xml b/man/sd_bus_is_open.xml index 5cd2d18c4d5..0388db82a63 100644 --- a/man/sd_bus_is_open.xml +++ b/man/sd_bus_is_open.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_is_open @@ -88,13 +89,7 @@ - - Notes - - sd_bus_is_open() and sd_bus_is_ready() are available as - a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. - + See Also diff --git a/man/sd_bus_path_encode.xml b/man/sd_bus_path_encode.xml index 4c60a8fa3ec..03130a697bb 100644 --- a/man/sd_bus_path_encode.xml +++ b/man/sd_bus_path_encode.xml @@ -6,7 +6,7 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_path_encode @@ -141,15 +141,7 @@ by the caller. - - Notes - - sd_bus_path_encode() and - sd_bus_path_decode() are available as a - shared library, which can be compiled and linked to with the - libsystemd pkg-config1 - file. - + See Also diff --git a/man/sd_bus_request_name.xml b/man/sd_bus_request_name.xml index 54a14c877c2..3c98b60c6ac 100644 --- a/man/sd_bus_request_name.xml +++ b/man/sd_bus_request_name.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_request_name @@ -193,13 +194,7 @@ - - Notes - - The sd_bus_acquire_name() and the other interfaces described here are available as a - shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. - + See Also diff --git a/man/sd_bus_set_connected_signal.xml b/man/sd_bus_set_connected_signal.xml index 9dd47bc4dfc..32fc630cfec 100644 --- a/man/sd_bus_set_connected_signal.xml +++ b/man/sd_bus_set_connected_signal.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_set_connected_signal @@ -94,13 +95,7 @@ - - Notes - - sd_bus_set_connected_signal() and sd_bus_get_connected_signal() are available as - a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. - + See Also diff --git a/man/sd_bus_set_sender.xml b/man/sd_bus_set_sender.xml index 512fffcf05e..556e72cefc8 100644 --- a/man/sd_bus_set_sender.xml +++ b/man/sd_bus_set_sender.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_set_sender @@ -89,13 +90,7 @@ - - Notes - - sd_bus_set_sender() and sd_bus_get_sender() are available as - a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. - + See Also diff --git a/man/sd_bus_set_watch_bind.xml b/man/sd_bus_set_watch_bind.xml index 41a4bc27dc4..129b98c5f3f 100644 --- a/man/sd_bus_set_watch_bind.xml +++ b/man/sd_bus_set_watch_bind.xml @@ -6,7 +6,8 @@ SPDX-License-Identifier: LGPL-2.1+ --> - + sd_bus_set_watch_bind @@ -100,13 +101,7 @@ - - Notes - - sd_bus_set_watch_bind() and sd_bus_get_watch_bind() are available as - a shared library, which can be compiled and linked to with the libsystemd pkg-config1 file. - + See Also