From c1d76521550a9e72d70436e7f4e6ab2fe1f033fc Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 16 Aug 2023 16:42:26 +0200 Subject: [PATCH] channel: Make the blocking parameters of the object queue explicit --- pdns/channel.hh | 20 +++++++++++++++----- pdns/delaypipe.cc | 2 +- pdns/distributor.hh | 2 +- pdns/dnsdist-tcp.cc | 6 +++--- pdns/dnsdistdist/dnsdist-nghttp2.cc | 2 +- pdns/dnsdistdist/doh.cc | 4 ++-- pdns/snmp-agent.cc | 2 +- pdns/test-channel.cc | 2 +- 8 files changed, 25 insertions(+), 15 deletions(-) diff --git a/pdns/channel.hh b/pdns/channel.hh index 6947d3b26d4b..2d848fc9eee3 100644 --- a/pdns/channel.hh +++ b/pdns/channel.hh @@ -43,6 +43,17 @@ namespace pdns { namespace channel { + enum class SenderBlockingMode + { + SenderNonBlocking, + SenderBlocking + }; + enum class ReceiverBlockingMode + { + ReceiverNonBlocking, + ReceiverBlocking + }; + /** * The sender's end of a channel used to pass objects between threads. * @@ -136,7 +147,7 @@ namespace channel * \throw runtime_error if the channel creation failed. */ template > - std::pair, Receiver> createObjectQueue(bool sendNonBlocking = true, bool receiveNonBlocking = true, size_t pipeBufferSize = 0, bool throwOnEOF = true); + std::pair, Receiver> createObjectQueue(SenderBlockingMode senderBlockingMode = SenderBlockingMode::SenderNonBlocking, ReceiverBlockingMode receiverBlockingMode = ReceiverBlockingMode::ReceiverNonBlocking, size_t pipeBufferSize = 0, bool throwOnEOF = true); /** * The notifier's end of a channel used to communicate between threads. @@ -307,7 +318,7 @@ namespace channel } template - std::pair, Receiver> createObjectQueue(bool sendNonBlocking, bool receiveNonBlocking, size_t pipeBufferSize, bool throwOnEOF) + std::pair, Receiver> createObjectQueue(SenderBlockingMode senderBlockingMode, ReceiverBlockingMode receiverBlockingMode, size_t pipeBufferSize, bool throwOnEOF) { int fds[2] = {-1, -1}; if (pipe(fds) < 0) { @@ -316,13 +327,12 @@ namespace channel FDWrapper sender(fds[1]); FDWrapper receiver(fds[0]); - - if (receiveNonBlocking && !setNonBlocking(receiver.getHandle())) { + if (receiverBlockingMode == ReceiverBlockingMode::ReceiverNonBlocking && !setNonBlocking(receiver.getHandle())) { int err = errno; throw std::runtime_error("Error making channel pipe non-blocking: " + stringerror(err)); } - if (sendNonBlocking && !setNonBlocking(sender.getHandle())) { + if (senderBlockingMode == SenderBlockingMode::SenderNonBlocking && !setNonBlocking(sender.getHandle())) { int err = errno; throw std::runtime_error("Error making channel pipe non-blocking: " + stringerror(err)); } diff --git a/pdns/delaypipe.cc b/pdns/delaypipe.cc index 645bcf56109f..ada096c1a27f 100644 --- a/pdns/delaypipe.cc +++ b/pdns/delaypipe.cc @@ -28,7 +28,7 @@ template ObjectPipe::ObjectPipe() { - auto [sender, receiver] = pdns::channel::createObjectQueue(false, true, 0, false); + auto [sender, receiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, 0, false); d_sender = std::move(sender); d_receiver = std::move(receiver); } diff --git a/pdns/distributor.hh b/pdns/distributor.hh index d021cfa0f71f..cdee87b67dc7 100644 --- a/pdns/distributor.hh +++ b/pdns/distributor.hh @@ -165,7 +165,7 @@ templateMultiThreadDistributor(false, false); + auto [sender, receiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderBlocking, pdns::channel::ReceiverBlockingMode::ReceiverBlocking); d_senders.push_back(std::move(sender)); d_receivers.push_back(std::move(receiver)); } diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index 2a1d7b80eebb..14af2564e32c 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -130,11 +130,11 @@ TCPClientCollection::TCPClientCollection(size_t maxThreads, std::vector& tcpAcceptStates) { try { - auto [queryChannelSender, queryChannelReceiver] = pdns::channel::createObjectQueue(true, true, g_tcpInternalPipeBufferSize); + auto [queryChannelSender, queryChannelReceiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, g_tcpInternalPipeBufferSize); - auto [crossProtocolQueryChannelSender, crossProtocolQueryChannelReceiver] = pdns::channel::createObjectQueue(true, true, g_tcpInternalPipeBufferSize); + auto [crossProtocolQueryChannelSender, crossProtocolQueryChannelReceiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, g_tcpInternalPipeBufferSize); - auto [crossProtocolResponseChannelSender, crossProtocolResponseChannelReceiver] = pdns::channel::createObjectQueue(true, true, g_tcpInternalPipeBufferSize); + auto [crossProtocolResponseChannelSender, crossProtocolResponseChannelReceiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, g_tcpInternalPipeBufferSize); vinfolog("Adding TCP Client thread"); diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.cc b/pdns/dnsdistdist/dnsdist-nghttp2.cc index fdc3d7a294c2..39e60009e05d 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2.cc @@ -1054,7 +1054,7 @@ void DoHClientCollection::addThread() { #ifdef HAVE_NGHTTP2 try { - auto [sender, receiver] = pdns::channel::createObjectQueue(true, true, g_tcpInternalPipeBufferSize); + auto [sender, receiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, g_tcpInternalPipeBufferSize); vinfolog("Adding DoH Client thread"); std::lock_guard lock(d_mutex); diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 2a24dabebca5..dac28e8ca921 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -175,14 +175,14 @@ struct DOHServerConfig { #ifndef USE_SINGLE_ACCEPTOR_THREAD { - auto [sender, receiver] = pdns::channel::createObjectQueue(true, false, internalPipeBufferSize); + auto [sender, receiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverBlocking, internalPipeBufferSize); d_querySender = std::move(sender); d_queryReceiver = std::move(receiver); } #endif /* USE_SINGLE_ACCEPTOR_THREAD */ { - auto [sender, receiver] = pdns::channel::createObjectQueue(true, true, internalPipeBufferSize); + auto [sender, receiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, internalPipeBufferSize); d_responseSender = std::move(sender); d_responseReceiver = std::move(receiver); } diff --git a/pdns/snmp-agent.cc b/pdns/snmp-agent.cc index d13d7d003040..2c9121c279ac 100644 --- a/pdns/snmp-agent.cc +++ b/pdns/snmp-agent.cc @@ -186,7 +186,7 @@ SNMPAgent::SNMPAgent([[maybe_unused]] const std::string& name, [[maybe_unused]] init_snmp(name.c_str()); - auto [sender, receiver] = pdns::channel::createObjectQueue(true, true); + auto [sender, receiver] = pdns::channel::createObjectQueue(); d_sender = std::move(sender); d_receiver = std::move(receiver); #endif /* HAVE_NET_SNMP */ diff --git a/pdns/test-channel.cc b/pdns/test-channel.cc index 9a7e4e037b58..8bd042122e04 100644 --- a/pdns/test-channel.cc +++ b/pdns/test-channel.cc @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(test_object_queue_throw_on_eof) BOOST_AUTO_TEST_CASE(test_object_queue_do_not_throw_on_eof) { - auto [sender, receiver] = pdns::channel::createObjectQueue(true, true, 0U, false); + auto [sender, receiver] = pdns::channel::createObjectQueue(pdns::channel::SenderBlockingMode::SenderNonBlocking, pdns::channel::ReceiverBlockingMode::ReceiverNonBlocking, 0U, false); sender.close(); auto got = receiver.receive(); BOOST_CHECK(got == std::nullopt);