From 6896b29a0f1a8972bda5a4fb46551a5437c443be Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Mon, 15 Jan 2024 11:50:30 +0100 Subject: [PATCH] Disable multiple receive threads by default On non-Windows platforms, the default was to use multiple receive threads, which in turn implied the use of blocking read operations. On termination, those blocking calls were interrupted by sending a packet to itself, but that only works if the firewall allows it. If the firewall drops these packet, the process hangs. Various people have run into this problem, changing to using a single thread always solves the problem. So, changing the default until a better solution for interrupting these calls is implemented is probably for the best. Signed-off-by: Erik Boasson --- docs/manual/config/config_file_reference.rst | 6 ++++-- docs/manual/options.md | 6 ++++-- etc/cyclonedds.rnc | 4 ++-- etc/cyclonedds.xsd | 4 ++-- src/core/ddsi/defconfig.c | 2 +- src/core/ddsi/src/ddsi__cfgelems.h | 8 ++++---- src/core/ddsi/src/ddsi_init.c | 20 +++++++------------- 7 files changed, 24 insertions(+), 26 deletions(-) diff --git a/docs/manual/config/config_file_reference.rst b/docs/manual/config/config_file_reference.rst index 54f77f7faa..a9c4280258 100644 --- a/docs/manual/config/config_file_reference.rst +++ b/docs/manual/config/config_file_reference.rst @@ -1161,7 +1161,9 @@ Attributes: :ref:`maxretries - + diff --git a/etc/cyclonedds.rnc b/etc/cyclonedds.rnc index 03fe0317c8..02ded851fa 100644 --- a/etc/cyclonedds.rnc +++ b/etc/cyclonedds.rnc @@ -559,7 +559,7 @@ CycloneDDS configuration""" ] ] xsd:integer }? & [ a:documentation [ xml:lang="en" """ -

This element controls whether all traffic is handled by a single receive thread (false) or whether multiple receive threads may be used to improve latency (true). By default it is disabled on Windows because it appears that one cannot count on being able to send packets to oneself, which is necessary to stop the thread during shutdown. Currently multiple receive threads are only used for connectionless transport (e.g., UDP) and ManySocketsMode not set to single (the default).

+

This element controls whether all traffic is handled by a single receive thread (false) or whether multiple receive threads may be used to improve latency (true). The value "default" currently maps to false because of firewalls potentially blocking the packets it sends to itself to interrupt the blocking reads during termination.

Currently multiple receive threads are only used for connectionless transport (e.g., UDP) and ManySocketsMode not set to single (the default).

The default value is: default

""" ] ] element MultipleReceiveThreads { [ a:documentation [ xml:lang="en" """ @@ -1285,7 +1285,7 @@ MIIEpAIBAAKCAQEA3HIh...AOBaaqSV37XBUJg==
} # generated from ddsi_config.h[570f67bd3080674a4bad53d9580a8bb7ad1e6e4d] # generated from ddsi__cfgunits.h[bd22f0c0ed210501d0ecd3b07c992eca549ef5aa] -# generated from ddsi__cfgelems.h[13337a006d5313519c88c3f3643f27992840cfd3] +# generated from ddsi__cfgelems.h[d4d0b8c7cf61f0a1cfa4b62e02458cf7b8962536] # generated from ddsi_config.c[efeae198a5e12ca8977a655216470564b5c44b64] # generated from _confgen.h[e32eabfc35e9f3a7dcb63b19ed148c0d17c6e5fc] # generated from _confgen.c[237308acd53897a34e8c643e16e05a61d73ffd65] diff --git a/etc/cyclonedds.xsd b/etc/cyclonedds.xsd index 5f8a20bb86..4ab6d375b9 100644 --- a/etc/cyclonedds.xsd +++ b/etc/cyclonedds.xsd @@ -883,7 +883,7 @@ CycloneDDS configuration -<p>This element controls whether all traffic is handled by a single receive thread (false) or whether multiple receive threads may be used to improve latency (true). By default it is disabled on Windows because it appears that one cannot count on being able to send packets to oneself, which is necessary to stop the thread during shutdown. Currently multiple receive threads are only used for connectionless transport (e.g., UDP) and ManySocketsMode not set to single (the default).</p> +<p>This element controls whether all traffic is handled by a single receive thread (false) or whether multiple receive threads may be used to improve latency (true). The value "default" currently maps to false because of firewalls potentially blocking the packets it sends to itself to interrupt the blocking reads during termination.</p><p>Currently multiple receive threads are only used for connectionless transport (e.g., UDP) and ManySocketsMode not set to single (the default).</p> <p>The default value is: <code>default</code></p> @@ -1941,7 +1941,7 @@ MIIEpAIBAAKCAQEA3HIh...AOBaaqSV37XBUJg==<br> - + diff --git a/src/core/ddsi/defconfig.c b/src/core/ddsi/defconfig.c index 17686b6ac6..814d0d5ef5 100644 --- a/src/core/ddsi/defconfig.c +++ b/src/core/ddsi/defconfig.c @@ -101,7 +101,7 @@ void ddsi_config_init_default (struct ddsi_config *cfg) } /* generated from ddsi_config.h[570f67bd3080674a4bad53d9580a8bb7ad1e6e4d] */ /* generated from ddsi__cfgunits.h[bd22f0c0ed210501d0ecd3b07c992eca549ef5aa] */ -/* generated from ddsi__cfgelems.h[13337a006d5313519c88c3f3643f27992840cfd3] */ +/* generated from ddsi__cfgelems.h[d4d0b8c7cf61f0a1cfa4b62e02458cf7b8962536] */ /* generated from ddsi_config.c[efeae198a5e12ca8977a655216470564b5c44b64] */ /* generated from _confgen.h[e32eabfc35e9f3a7dcb63b19ed148c0d17c6e5fc] */ /* generated from _confgen.c[237308acd53897a34e8c643e16e05a61d73ffd65] */ diff --git a/src/core/ddsi/src/ddsi__cfgelems.h b/src/core/ddsi/src/ddsi__cfgelems.h index aaa64fa0da..a0ccf089fa 100644 --- a/src/core/ddsi/src/ddsi__cfgelems.h +++ b/src/core/ddsi/src/ddsi__cfgelems.h @@ -1525,10 +1525,10 @@ static struct cfgelem internal_cfgelems[] = { DESCRIPTION( "

This element controls whether all traffic is handled by a single " "receive thread (false) or whether multiple receive threads may be used " - "to improve latency (true). By default it is disabled on Windows because " - "it appears that one cannot count on being able to send packets to " - "oneself, which is necessary to stop the thread during shutdown. " - "Currently multiple receive threads are only used for connectionless " + "to improve latency (true). The value \"default\" currently maps to " + "false because of firewalls potentially blocking the packets it sends " + "to itself to interrupt the blocking reads during termination.

" + "

Currently multiple receive threads are only used for connectionless " "transport (e.g., UDP) and ManySocketsMode not set to single (the " "default).

"), VALUES("false","true","default")), diff --git a/src/core/ddsi/src/ddsi_init.c b/src/core/ddsi/src/ddsi_init.c index d662e8b469..4c1820fad4 100644 --- a/src/core/ddsi/src/ddsi_init.c +++ b/src/core/ddsi/src/ddsi_init.c @@ -905,25 +905,19 @@ static void make_special_types (struct ddsi_domaingv *gv) static bool use_multiple_receive_threads (const struct ddsi_config *cfg) { - /* Under some unknown circumstances Windows (at least Windows 10) exhibits - the interesting behaviour of losing its ability to let us send packets - to our own sockets. When that happens, dedicated receive threads can no - longer be stopped and Cyclone hangs in shutdown. So until someone - figures out why this happens, it is probably best have a different - default on Windows. */ -#if _WIN32 - const bool def = false; -#else - const bool def = true; -#endif switch (cfg->multiple_recv_threads) { case DDSI_BOOLDEF_FALSE: + case DDSI_BOOLDEF_DEFAULT: + // Too many people run into trouble with firewalls blocking the packets + // Cyclone sends to itself for interrupting the blocking reads. So + // default to a single thread and multiplexing. + // + // (One could also consider multiple threads, but still doing select+read + // but having fewer threads is arguably a good thing in itself.) return false; case DDSI_BOOLDEF_TRUE: return true; - case DDSI_BOOLDEF_DEFAULT: - return def; } assert (0); return false;