From b3c4490d27e85cf138dcf6a56ec08b57c9016f0d Mon Sep 17 00:00:00 2001 From: "Paul E. Jones" Date: Tue, 5 Sep 2023 16:20:10 -0400 Subject: [PATCH] Changes to support picoquic logging callbacks --- src/transport_picoquic.cpp | 117 ++----------------------------------- src/transport_picoquic.h | 16 +++-- 2 files changed, 16 insertions(+), 117 deletions(-) diff --git a/src/transport_picoquic.cpp b/src/transport_picoquic.cpp index 66d8cb9..415a266 100644 --- a/src/transport_picoquic.cpp +++ b/src/transport_picoquic.cpp @@ -459,7 +459,6 @@ PicoQuicTransport::PicoQuicTransport(const TransportRemote& server, bool _is_server_mode, const cantina::LoggerPointer& logger) : logger(std::make_shared("QUIC", logger)) - , opened_logging_fds(false) , _is_server_mode(_is_server_mode) , stop(false) , transportStatus(TransportStatus::Connecting) @@ -519,32 +518,10 @@ PicoQuicTransport::shutdown() picoquic_config_clear(&config); - // Cleanup picoquic logging thread and descriptors - if (opened_logging_fds) - { - std::uint8_t zero{}; - - // Ensure no logs are directed to logging streams - debug_set_stream(stdout); - - // Wake up the thread waiting on logging pipe fd by writing zero octet - write(logging_fds[1], &zero, 1); - - // Wait for the thread to end - if (logging_thread.joinable()) { - logger->Log("Shutting down logging thread"); - logging_thread.join(); - } - - // Close the logfp (this will close logging_fds[1]) - fclose(logfp); - - // Close logging file descriptors - close(logging_fds[0]); - close(logging_fds[1]); // redundant - - // Clear the flag indicating open fds - opened_logging_fds = false; + // If logging picoquic events, stop those + if (picoquic_logger) { + debug_set_callback(NULL, NULL); + picoquic_logger.reset(); } } @@ -599,38 +576,8 @@ PicoQuicTransport::start() uint64_t current_time = picoquic_current_time(); if (debug) { - if (!opened_logging_fds) - { - // Open pipe for reading and writing - if (pipe(logging_fds) == 0) { - opened_logging_fds = true; - - // Create a FILE object to provide to picoquic - logfp = fdopen(logging_fds[1], "w"); - if (logfp) { - logging_thread = - std::thread(&PicoQuicTransport::PicoQuicLogging, this); - debug_set_stream(logfp); // Enable picoquic debug to logfp - } else { - logger->error << "fdopen() failed for pipe" << std::flush; - - // Close logging file descriptors - close(logging_fds[0]); - close(logging_fds[1]); - opened_logging_fds = false; - } - } - else - { - logger->error << "Failed to open pipe for picoquic logging" - << std::flush; - close(logging_fds[0]); - close(logging_fds[1]); - } - } - - // If no other place to direct logs, send them to stdout - if (!opened_logging_fds) debug_set_stream(stdout); + picoquic_logger = std::make_shared("PQIC", logger); + debug_set_callback(&PicoQuicTransport::PicoQuicLogging, this); } (void)picoquic_config_set_option(&config, picoquic_option_CC_ALGO, "bbr"); @@ -687,58 +634,6 @@ PicoQuicTransport::start() return cid; } -void PicoQuicTransport::PicoQuicLogging() -{ - std::array buffer; - ssize_t octets_read; - fd_set pipe_fds; - - cantina::LoggerPointer pico_logger = - std::make_shared("PQIC", logger); - - while (!stop) { - // Prepare file descriptor to read - FD_ZERO(&pipe_fds); - FD_SET(logging_fds[0], &pipe_fds); - - // Wait until something is ready to read - if (select(logging_fds[0] + 1, - &pipe_fds, - nullptr, - nullptr, - nullptr) == -1) { - pico_logger->error << "Select for logging failed" << std::flush; - // Set picoquic debug to stdout - debug_set_stream(stdout); - break; - } - - // Told to stop? - if (stop) break; - - // Data to read? - if (FD_ISSET(logging_fds[0], &pipe_fds)) { - octets_read = read(logging_fds[0], buffer.data(), buffer.size()); - if (octets_read > 0) - { - for (std::size_t i = 0; i < octets_read; i++) { - switch (buffer[i]) { - case '\n': - pico_logger->info << std::flush; - break; - case '\0': - // Do not output null characters - break; - default: - pico_logger->info << buffer[i]; - break; - } - } - } - } - } -} - void PicoQuicTransport::pq_runner() { while (auto cb = std::move(picoquic_runner_queue.pop())) { diff --git a/src/transport_picoquic.h b/src/transport_picoquic.h index 5c24aee..b63366f 100644 --- a/src/transport_picoquic.h +++ b/src/transport_picoquic.h @@ -95,6 +95,15 @@ class PicoQuicTransport : public ITransport using Exception::Exception; }; + static void PicoQuicLogging(const char *message, void *argp) + { + auto instance = reinterpret_cast(argp); + if (!instance->stop && instance->picoquic_logger) + { + instance->picoquic_logger->Log(message); + } + } + public: PicoQuicTransport(const TransportRemote& server, const TransportConfig& tcfg, @@ -166,18 +175,13 @@ class PicoQuicTransport : public ITransport * Internal Public Variables */ cantina::LoggerPointer logger; - int logging_fds[2]; - bool opened_logging_fds; - FILE *logfp; - std::function logging_callback; - std::thread logging_thread; + cantina::LoggerPointer picoquic_logger; bool _is_server_mode; bool _is_unidirectional{ false }; bool debug {false}; private: - void PicoQuicLogging(); TransportContextId createClient(); void shutdown();