From c8e567dc4d0d1f145bce62126635b84d09c4b9fe Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 17:13:27 +0300 Subject: [PATCH 1/8] Moved callbacks to anonymous functions local to the translation unit. --- Pcap++/header/PcapLiveDevice.h | 4 - Pcap++/src/PcapLiveDevice.cpp | 149 +++++++++++++++++++++++---------- 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/Pcap++/header/PcapLiveDevice.h b/Pcap++/header/PcapLiveDevice.h index 14eebb0ca..b009578d6 100644 --- a/Pcap++/header/PcapLiveDevice.h +++ b/Pcap++/header/PcapLiveDevice.h @@ -165,10 +165,6 @@ namespace pcpp // threads void captureThreadMain(); - static void onPacketArrives(uint8_t* user, const struct pcap_pkthdr* pkthdr, const uint8_t* packet); - static void onPacketArrivesNoCallback(uint8_t* user, const struct pcap_pkthdr* pkthdr, const uint8_t* packet); - static void onPacketArrivesBlockingMode(uint8_t* user, const struct pcap_pkthdr* pkthdr, const uint8_t* packet); - public: /// The type of the live device enum LiveDeviceType diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 9df29c2b9..7c03b0cc7 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -340,54 +340,82 @@ namespace pcpp } } - void PcapLiveDevice::onPacketArrives(uint8_t* user, const struct pcap_pkthdr* pkthdr, const uint8_t* packet) + namespace { - PcapLiveDevice* pThis = reinterpret_cast(user); - if (pThis == nullptr) + struct CaptureContext { - PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); - return; - } + PcapLiveDevice* device; + OnPacketArrivesCallback callback; + void* userCookie; + }; - RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, pThis->getLinkType()); + struct AccumulatorCaptureContext + { + PcapLiveDevice* device; + RawPacketVector* capturedPackets; + }; - if (pThis->m_cbOnPacketArrives != nullptr) - pThis->m_cbOnPacketArrives(&rawPacket, pThis, pThis->m_cbOnPacketArrivesUserCookie); - } + struct CaptureContextST + { + PcapLiveDevice* device; + OnPacketArrivesStopBlocking callback; + void* userCookie; + bool requestStop = false; + }; - void PcapLiveDevice::onPacketArrivesNoCallback(uint8_t* user, const struct pcap_pkthdr* pkthdr, - const uint8_t* packet) - { - PcapLiveDevice* pThis = reinterpret_cast(user); - if (pThis == nullptr) + void onPacketArrivesCallback(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) { - PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); - return; - } + CaptureContext* context = reinterpret_cast(user); + if (context == nullptr || context->device == nullptr) + { + PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); + return; + } - uint8_t* packetData = new uint8_t[pkthdr->caplen]; - memcpy(packetData, packet, pkthdr->caplen); - RawPacket* rawPacketPtr = new RawPacket(packetData, pkthdr->caplen, pkthdr->ts, true, pThis->getLinkType()); - pThis->m_CapturedPackets->pushBack(rawPacketPtr); - } + RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); + if (context->callback != nullptr) + context->callback(&rawPacket, context->device, context->userCookie); + } - void PcapLiveDevice::onPacketArrivesBlockingMode(uint8_t* user, const struct pcap_pkthdr* pkthdr, - const uint8_t* packet) - { - PcapLiveDevice* pThis = reinterpret_cast(user); - if (pThis == nullptr) + void onPacketArrivesAccumulator(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) { - PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); - return; + AccumulatorCaptureContext* context = reinterpret_cast(user); + if (context == nullptr || context->device == nullptr || context->capturedPackets == nullptr) + { + PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance or captured packets vector"); + return; + } + + uint8_t* packetData = new uint8_t[pkthdr->caplen]; + std::memcpy(packetData, packet, pkthdr->caplen); + auto rawPacket = std::make_unique(packetData, pkthdr->caplen, pkthdr->ts, true, + context->device->getLinkType()); + context->capturedPackets->pushBack(std::move(rawPacket)); } - RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, pThis->getLinkType()); + void onPacketArrivesCallbackWithCancellation(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) + { + CaptureContextST* context = reinterpret_cast(user); + + if (context == nullptr || context->device == nullptr) + { + PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); + return; + } + + RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); - if (pThis->m_cbOnPacketArrivesBlockingMode != nullptr) - if (pThis->m_cbOnPacketArrivesBlockingMode(&rawPacket, pThis, - pThis->m_cbOnPacketArrivesBlockingModeUserCookie)) - pThis->m_StopThread = true; - } + if (context->callback != nullptr) + { + if (context->callback(&rawPacket, context->device, context->userCookie)) + { + // If the callback returns true, it means that the user wants to stop the capture + context->requestStop = true; + return; + } + } + } + } // namespace void PcapLiveDevice::captureThreadMain() { @@ -396,9 +424,15 @@ namespace pcpp if (m_CaptureCallbackMode) { + CaptureContext context; + context.device = this; + context.callback = m_cbOnPacketArrives; + context.userCookie = m_cbOnPacketArrivesUserCookie; + while (!m_StopThread) { - if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrives, reinterpret_cast(this)) == -1) + if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesCallback, + reinterpret_cast(&context)) == -1) { PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); m_StopThread = true; @@ -407,10 +441,14 @@ namespace pcpp } else { + AccumulatorCaptureContext context; + context.device = this; + context.capturedPackets = m_CapturedPackets; + while (!m_StopThread) { - if (pcap_dispatch(m_PcapDescriptor.get(), 100, onPacketArrivesNoCallback, - reinterpret_cast(this)) == -1) + if (pcap_dispatch(m_PcapDescriptor.get(), 100, onPacketArrivesAccumulator, + reinterpret_cast(&context)) == -1) { PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); m_StopThread = true; @@ -784,17 +822,28 @@ namespace pcpp bool shouldReturnError = false; + CaptureContextST context; + context.device = this; + context.callback = m_cbOnPacketArrivesBlockingMode; + context.userCookie = m_cbOnPacketArrivesBlockingModeUserCookie; + context.requestStop = false; + if (timeoutMs <= 0) { while (!m_StopThread) { - if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesBlockingMode, - reinterpret_cast(this)) == -1) + if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesCallbackWithCancellation, + reinterpret_cast(&context)) == -1) { PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); shouldReturnError = true; m_StopThread = true; } + else if (context.requestStop) + { + // If the callback requested to stop the capture, we break the loop + m_StopThread = true; + } } } else @@ -816,13 +865,18 @@ namespace pcpp if (ready > 0) { - if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesBlockingMode, - reinterpret_cast(this)) == -1) + if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesCallbackWithCancellation, + reinterpret_cast(&context)) == -1) { PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); shouldReturnError = true; m_StopThread = true; } + else if (context.requestStop) + { + // If the callback requested to stop the capture, we break the loop + m_StopThread = true; + } } else if (ready < 0) { @@ -838,13 +892,18 @@ namespace pcpp } else { - if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesBlockingMode, - reinterpret_cast(this)) == -1) + if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesCallbackWithCancellation, + reinterpret_cast(&context)) == -1) { PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); shouldReturnError = true; m_StopThread = true; } + else if (context.requestStop) + { + // If the callback requested to stop the capture, we break the loop + m_StopThread = true; + } } currentTime = std::chrono::steady_clock::now(); } From 9ed6ee951229408d14a05fe545d883fa8ae84501 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 17:19:51 +0300 Subject: [PATCH 2/8] Moved user callback validity checks to the start of the callbacks. --- Pcap++/src/PcapLiveDevice.cpp | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 7c03b0cc7..c4093a6dd 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -366,15 +366,14 @@ namespace pcpp void onPacketArrivesCallback(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) { CaptureContext* context = reinterpret_cast(user); - if (context == nullptr || context->device == nullptr) + if (context == nullptr || context->device == nullptr || context->callback == nullptr) { - PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); + PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance or callback"); return; } RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); - if (context->callback != nullptr) - context->callback(&rawPacket, context->device, context->userCookie); + context->callback(&rawPacket, context->device, context->userCookie); } void onPacketArrivesAccumulator(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) @@ -397,22 +396,25 @@ namespace pcpp { CaptureContextST* context = reinterpret_cast(user); - if (context == nullptr || context->device == nullptr) + if (context == nullptr || context->device == nullptr || context->callback) { - PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); + PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance or callback"); + return; + } + + if (context->requestStop) + { + // If requestStop is true, there is no need to process the packet + PCPP_LOG_DEBUG("Capture request stop is set, skipping packet processing"); return; } RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); - if (context->callback != nullptr) + if (context->callback(&rawPacket, context->device, context->userCookie)) { - if (context->callback(&rawPacket, context->device, context->userCookie)) - { - // If the callback returns true, it means that the user wants to stop the capture - context->requestStop = true; - return; - } + // If the callback returns true, it means that the user wants to stop the capture + context->requestStop = true; } } } // namespace From 1954ebc8b99a16b05667c2aef1b7d6e77ccf639b Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 17:40:03 +0300 Subject: [PATCH 3/8] Changed captureThreadMain to an internal function. --- Pcap++/header/PcapLiveDevice.h | 3 -- Pcap++/src/PcapLiveDevice.cpp | 64 ++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/Pcap++/header/PcapLiveDevice.h b/Pcap++/header/PcapLiveDevice.h index b009578d6..8d19e5571 100644 --- a/Pcap++/header/PcapLiveDevice.h +++ b/Pcap++/header/PcapLiveDevice.h @@ -162,9 +162,6 @@ namespace pcpp void setDeviceMacAddress(); void setDefaultGateway(); - // threads - void captureThreadMain(); - public: /// The type of the live device enum LiveDeviceType diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index c4093a6dd..3ab865cba 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -417,48 +417,46 @@ namespace pcpp context->requestStop = true; } } - } // namespace - void PcapLiveDevice::captureThreadMain() - { - PCPP_LOG_DEBUG("Started capture thread for device '" << m_InterfaceDetails.name << "'"); - m_CaptureThreadStarted = true; - - if (m_CaptureCallbackMode) + void captureThreadMain(std::atomic_bool& stopFlag, std::atomic_bool& isRunningFlag, + internal::PcapHandle const& pcapDescriptor, CaptureContext context) { - CaptureContext context; - context.device = this; - context.callback = m_cbOnPacketArrives; - context.userCookie = m_cbOnPacketArrivesUserCookie; + PCPP_LOG_DEBUG("Started capture thread for device '" << context.device->getName() << "'"); + isRunningFlag.store(true); - while (!m_StopThread) + while (!stopFlag.load()) { - if (pcap_dispatch(m_PcapDescriptor.get(), -1, onPacketArrivesCallback, + if (pcap_dispatch(pcapDescriptor.get(), -1, onPacketArrivesCallback, reinterpret_cast(&context)) == -1) { - PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); - m_StopThread = true; + PCPP_LOG_ERROR("pcap_dispatch returned an error: " << pcapDescriptor.getLastError()); + stopFlag.store(true); } } + + isRunningFlag.store(false); + PCPP_LOG_DEBUG("Ended capture thread for device '" << context.device->getName() << "'"); } - else - { - AccumulatorCaptureContext context; - context.device = this; - context.capturedPackets = m_CapturedPackets; - while (!m_StopThread) + void captureThreadMainAccumulator(std::atomic_bool& stopFlag, std::atomic_bool& isRunningFlag, + internal::PcapHandle const& pcapDescriptor, AccumulatorCaptureContext context) + { + PCPP_LOG_DEBUG("Started capture thread for device '" << context.device->getName() << "'"); + isRunningFlag.store(true); + while (!stopFlag.load()) { - if (pcap_dispatch(m_PcapDescriptor.get(), 100, onPacketArrivesAccumulator, + if (pcap_dispatch(pcapDescriptor.get(), 100, onPacketArrivesAccumulator, reinterpret_cast(&context)) == -1) { - PCPP_LOG_ERROR("pcap_dispatch returned an error: " << m_PcapDescriptor.getLastError()); - m_StopThread = true; + PCPP_LOG_ERROR("pcap_dispatch returned an error: " << pcapDescriptor.getLastError()); + stopFlag.store(true); } } + + isRunningFlag.store(false); + PCPP_LOG_DEBUG("Ended capture thread for device '" << context.device->getName() << "'"); } - PCPP_LOG_DEBUG("Ended capture thread for device '" << m_InterfaceDetails.name << "'"); - } + } // namespace internal::PcapHandle PcapLiveDevice::doOpen(const DeviceConfiguration& config) { @@ -709,10 +707,12 @@ namespace pcpp } m_CaptureCallbackMode = true; - m_cbOnPacketArrives = std::move(onPacketArrives); - m_cbOnPacketArrivesUserCookie = onPacketArrivesUserCookie; + CaptureContext context; + context.device = this; + context.callback = std::move(onPacketArrives); + context.userCookie = onPacketArrivesUserCookie; - m_CaptureThread = std::thread(&pcpp::PcapLiveDevice::captureThreadMain, this); + m_CaptureThread = std::thread(&captureThreadMain, std::ref(m_StopThread), std::ref(m_CaptureThreadStarted), std::ref(m_PcapDescriptor), std::move(context)); // Wait thread to be start // C++20 = m_CaptureThreadStarted.wait(true); @@ -764,7 +764,11 @@ namespace pcpp m_CapturedPackets->clear(); m_CaptureCallbackMode = false; - m_CaptureThread = std::thread(&pcpp::PcapLiveDevice::captureThreadMain, this); + AccumulatorCaptureContext context; + context.device = this; + context.capturedPackets = &capturedPacketsVector; + m_CaptureThread = std::thread(&captureThreadMainAccumulator, std::ref(m_StopThread), + std::ref(m_CaptureThreadStarted), std::ref(m_PcapDescriptor), std::move(context)); // Wait thread to be start // C++20 = m_CaptureThreadStarted.wait(true); while (m_CaptureThreadStarted != true) From e02c6b483e5f9bbb48803b2f1d5b5ac032adc53b Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 18:43:22 +0300 Subject: [PATCH 4/8] Fixed callback for statistics only capture. Removed redundant fields. --- Pcap++/header/PcapLiveDevice.h | 6 ----- Pcap++/src/PcapLiveDevice.cpp | 42 ++++++++++++++-------------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/Pcap++/header/PcapLiveDevice.h b/Pcap++/header/PcapLiveDevice.h index 8d19e5571..80f7610ce 100644 --- a/Pcap++/header/PcapLiveDevice.h +++ b/Pcap++/header/PcapLiveDevice.h @@ -141,12 +141,6 @@ namespace pcpp // Should be set to true by the Callee for the Caller std::atomic m_CaptureThreadStarted; - OnPacketArrivesCallback m_cbOnPacketArrives; - void* m_cbOnPacketArrivesUserCookie; - OnPacketArrivesStopBlocking m_cbOnPacketArrivesBlockingMode; - void* m_cbOnPacketArrivesBlockingModeUserCookie; - RawPacketVector* m_CapturedPackets; - bool m_CaptureCallbackMode; LinkLayerType m_LinkType; bool m_UsePoll; diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 3ab865cba..2c2e29064 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -327,12 +327,6 @@ namespace pcpp m_CaptureThreadStarted = false; m_StopThread = false; m_CaptureThread = {}; - m_cbOnPacketArrives = nullptr; - m_cbOnPacketArrivesBlockingMode = nullptr; - m_cbOnPacketArrivesBlockingModeUserCookie = nullptr; - m_cbOnPacketArrivesUserCookie = nullptr; - m_CaptureCallbackMode = true; - m_CapturedPackets = nullptr; if (calculateMacAddress) { setDeviceMacAddress(); @@ -363,6 +357,9 @@ namespace pcpp bool requestStop = false; }; + void onPacketArrivesNoop(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) + {} + void onPacketArrivesCallback(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) { CaptureContext* context = reinterpret_cast(user); @@ -396,7 +393,7 @@ namespace pcpp { CaptureContextST* context = reinterpret_cast(user); - if (context == nullptr || context->device == nullptr || context->callback) + if (context == nullptr || context->device == nullptr || context->callback == nullptr) { PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance or callback"); return; @@ -424,10 +421,14 @@ namespace pcpp PCPP_LOG_DEBUG("Started capture thread for device '" << context.device->getName() << "'"); isRunningFlag.store(true); + // If the callback is null, we use a no-op handler to avoid unnecessary overhead + // Statistics only capture still requires pcap_dispatch to be called, but we don't need to process packets. + pcap_handler callbackHandler = context.callback ? onPacketArrivesCallback : onPacketArrivesNoop; + while (!stopFlag.load()) { - if (pcap_dispatch(pcapDescriptor.get(), -1, onPacketArrivesCallback, - reinterpret_cast(&context)) == -1) + if (pcap_dispatch(pcapDescriptor.get(), -1, callbackHandler, reinterpret_cast(&context)) == + -1) { PCPP_LOG_ERROR("pcap_dispatch returned an error: " << pcapDescriptor.getLastError()); stopFlag.store(true); @@ -706,13 +707,13 @@ namespace pcpp return false; } - m_CaptureCallbackMode = true; CaptureContext context; context.device = this; context.callback = std::move(onPacketArrives); context.userCookie = onPacketArrivesUserCookie; - m_CaptureThread = std::thread(&captureThreadMain, std::ref(m_StopThread), std::ref(m_CaptureThreadStarted), std::ref(m_PcapDescriptor), std::move(context)); + m_CaptureThread = std::thread(&captureThreadMain, std::ref(m_StopThread), std::ref(m_CaptureThreadStarted), + std::ref(m_PcapDescriptor), std::move(context)); // Wait thread to be start // C++20 = m_CaptureThreadStarted.wait(true); @@ -760,10 +761,8 @@ namespace pcpp return false; } - m_CapturedPackets = &capturedPacketsVector; - m_CapturedPackets->clear(); + capturedPacketsVector.clear(); - m_CaptureCallbackMode = false; AccumulatorCaptureContext context; context.device = this; context.capturedPackets = &capturedPacketsVector; @@ -806,11 +805,6 @@ namespace pcpp PCPP_LOG_ERROR("Failed to prepare capture: " << ex.what()); return 0; } - m_cbOnPacketArrives = nullptr; - m_cbOnPacketArrivesUserCookie = nullptr; - - m_cbOnPacketArrivesBlockingMode = std::move(onPacketArrives); - m_cbOnPacketArrivesBlockingModeUserCookie = userCookie; m_CaptureThreadStarted = true; m_StopThread = false; @@ -830,8 +824,8 @@ namespace pcpp CaptureContextST context; context.device = this; - context.callback = m_cbOnPacketArrivesBlockingMode; - context.userCookie = m_cbOnPacketArrivesBlockingModeUserCookie; + context.callback = std::move(onPacketArrives); + context.userCookie = userCookie; context.requestStop = false; if (timeoutMs <= 0) @@ -917,8 +911,6 @@ namespace pcpp m_CaptureThreadStarted = false; m_StopThread = false; - m_cbOnPacketArrivesBlockingMode = nullptr; - m_cbOnPacketArrivesBlockingModeUserCookie = nullptr; if (shouldReturnError) { @@ -934,8 +926,8 @@ namespace pcpp void PcapLiveDevice::stopCapture() { - // in blocking mode stop capture isn't relevant - if (m_cbOnPacketArrivesBlockingMode != nullptr) + // In blocking mode, there is no capture thread, so we don't need to stop it + if (!m_CaptureThread.joinable()) return; m_StopThread = true; From 496938e953e11e9e538342d1a8421d574e49f073 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 18:47:30 +0300 Subject: [PATCH 5/8] Changed isRunning flag to hasStarted. --- Pcap++/src/PcapLiveDevice.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 2c2e29064..27097039e 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -415,11 +415,11 @@ namespace pcpp } } - void captureThreadMain(std::atomic_bool& stopFlag, std::atomic_bool& isRunningFlag, + void captureThreadMain(std::atomic_bool& stopFlag, std::atomic_bool& hasStarted, internal::PcapHandle const& pcapDescriptor, CaptureContext context) { PCPP_LOG_DEBUG("Started capture thread for device '" << context.device->getName() << "'"); - isRunningFlag.store(true); + hasStarted.store(true); // If the callback is null, we use a no-op handler to avoid unnecessary overhead // Statistics only capture still requires pcap_dispatch to be called, but we don't need to process packets. @@ -435,15 +435,14 @@ namespace pcpp } } - isRunningFlag.store(false); PCPP_LOG_DEBUG("Ended capture thread for device '" << context.device->getName() << "'"); } - void captureThreadMainAccumulator(std::atomic_bool& stopFlag, std::atomic_bool& isRunningFlag, + void captureThreadMainAccumulator(std::atomic_bool& stopFlag, std::atomic_bool& hasStarted, internal::PcapHandle const& pcapDescriptor, AccumulatorCaptureContext context) { PCPP_LOG_DEBUG("Started capture thread for device '" << context.device->getName() << "'"); - isRunningFlag.store(true); + hasStarted.store(true); while (!stopFlag.load()) { if (pcap_dispatch(pcapDescriptor.get(), 100, onPacketArrivesAccumulator, @@ -454,7 +453,6 @@ namespace pcpp } } - isRunningFlag.store(false); PCPP_LOG_DEBUG("Ended capture thread for device '" << context.device->getName() << "'"); } } // namespace From 312247b9eae0460c7c8b81af788d0d02c8def2a4 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 23:50:00 +0300 Subject: [PATCH 6/8] Replaced statistics worker with a procedure. --- Pcap++/header/PcapLiveDevice.h | 43 +------------ Pcap++/src/PcapLiveDevice.cpp | 107 ++++++++++++--------------------- 2 files changed, 40 insertions(+), 110 deletions(-) diff --git a/Pcap++/header/PcapLiveDevice.h b/Pcap++/header/PcapLiveDevice.h index 80f7610ce..a5f5947d2 100644 --- a/Pcap++/header/PcapLiveDevice.h +++ b/Pcap++/header/PcapLiveDevice.h @@ -82,44 +82,6 @@ namespace pcpp bool isLoopback; }; - /// @brief A worker thread that periodically calls the provided callback with updated statistics. - class StatisticsUpdateWorker - { - public: - /// @brief Constructs and starts a worker thread that periodically calls the provided callback with updated - /// statistics. - /// @param pcapDevice A pointer to the PcapLiveDevice instance to be monitored. - /// @param onStatsUpdateCallback A callback function to be called with updated statistics. - /// @param onStatsUpdateUserCookie A user-defined pointer that is passed to the callback function. - /// @param updateIntervalMs The interval in milliseconds between each callback invocation. - StatisticsUpdateWorker(PcapLiveDevice const& pcapDevice, OnStatsUpdateCallback onStatsUpdateCallback, - void* onStatsUpdateUserCookie = nullptr, unsigned int updateIntervalMs = 1000); - - /// @brief Stops the worker thread. - void stopWorker(); - - private: - struct ThreadData - { - PcapLiveDevice const* pcapDevice = nullptr; - OnStatsUpdateCallback cbOnStatsUpdate; - void* cbOnStatsUpdateUserCookie = nullptr; - unsigned int updateIntervalMs = 1000; // Default update interval is 1 second - }; - - struct SharedThreadData - { - std::atomic_bool stopRequested{ false }; - }; - - /// @brief Main function for the worker thread. - /// @remarks This function is static to allow the worker class to be movable. - static void workerMain(std::shared_ptr sharedThreadData, ThreadData threadData); - - std::shared_ptr m_SharedThreadData; - std::thread m_WorkerThread; - }; - // This is a second descriptor for the same device. It is needed because of a bug // that occurs in libpcap on Linux (on Windows using WinPcap/Npcap it works well): // It's impossible to capture packets sent by the same descriptor @@ -131,10 +93,9 @@ namespace pcpp uint32_t m_DeviceMtu; MacAddress m_MacAddress; IPv4Address m_DefaultGateway; - std::thread m_CaptureThread; - // TODO: Cpp17 Using std::optional might be better here - std::unique_ptr m_StatisticsUpdateWorker; + std::thread m_CaptureThread; + std::thread m_StatsThread; // Should be set to true by the Caller for the Callee std::atomic m_StopThread; diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 27097039e..a7bf49677 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -230,68 +230,6 @@ namespace pcpp } } - PcapLiveDevice::StatisticsUpdateWorker::StatisticsUpdateWorker(PcapLiveDevice const& pcapDevice, - OnStatsUpdateCallback onStatsUpdateCallback, - void* onStatsUpdateUserCookie, - unsigned int updateIntervalMs) - { - // Setup thread data - m_SharedThreadData = std::make_shared(); - - ThreadData threadData; - threadData.pcapDevice = &pcapDevice; - threadData.cbOnStatsUpdate = onStatsUpdateCallback; - threadData.cbOnStatsUpdateUserCookie = onStatsUpdateUserCookie; - threadData.updateIntervalMs = updateIntervalMs; - - // Start the thread - m_WorkerThread = std::thread(&StatisticsUpdateWorker::workerMain, m_SharedThreadData, std::move(threadData)); - } - - void PcapLiveDevice::StatisticsUpdateWorker::stopWorker() - { - m_SharedThreadData->stopRequested = true; - if (m_WorkerThread.joinable()) - { - m_WorkerThread.join(); - } - } - - void PcapLiveDevice::StatisticsUpdateWorker::workerMain(std::shared_ptr sharedThreadData, - ThreadData threadData) - { - if (sharedThreadData == nullptr) - { - PCPP_LOG_ERROR("Shared thread data is null"); - return; - } - - if (threadData.pcapDevice == nullptr) - { - PCPP_LOG_ERROR("Pcap device is null"); - return; - } - - if (threadData.cbOnStatsUpdate == nullptr) - { - PCPP_LOG_ERROR("Statistics Callback is null"); - return; - } - - PCPP_LOG_DEBUG("Started statistics thread"); - - PcapStats stats; - auto sleepDuration = std::chrono::milliseconds(threadData.updateIntervalMs); - while (!sharedThreadData->stopRequested) - { - threadData.pcapDevice->getStatistics(stats); - threadData.cbOnStatsUpdate(stats, threadData.cbOnStatsUpdateUserCookie); - std::this_thread::sleep_for(sleepDuration); - } - - PCPP_LOG_DEBUG("Stopped statistics thread"); - } - PcapLiveDevice::PcapLiveDevice(DeviceInterfaceDetails interfaceDetails, bool calculateMTU, bool calculateMacAddress, bool calculateDefaultGateway) : IPcapDevice(), m_PcapSendDescriptor(nullptr), m_PcapSelectableFd(-1), @@ -336,6 +274,35 @@ namespace pcpp namespace { + struct StatisticsUpdateContext + { + OnStatsUpdateCallback cbOnStatsUpdate; + void* cbOnStatsUpdateUserCookie = nullptr; + unsigned int updateIntervalMs = 1000; // Default update interval is 1 second + }; + + void statsThreadMain(std::atomic_bool& stopFlag, internal::PcapHandle const& pcapDescriptor, + StatisticsUpdateContext context) + { + if (context.cbOnStatsUpdate == nullptr) + { + PCPP_LOG_ERROR("No callback provided for statistics updates"); + return; + } + + PCPP_LOG_DEBUG("Started statistics thread"); + + IPcapDevice::PcapStats stats; + auto sleepDuration = std::chrono::milliseconds(context.updateIntervalMs); + while (!stopFlag.load()) + { + pcapDescriptor.getStatistics(stats); + context.cbOnStatsUpdate(stats, context.cbOnStatsUpdateUserCookie); + std::this_thread::sleep_for(sleepDuration); + } + PCPP_LOG_DEBUG("Ended statistics thread"); + } + struct CaptureContext { PcapLiveDevice* device; @@ -724,10 +691,13 @@ namespace pcpp if (onStatsUpdate != nullptr && intervalInSecondsToUpdateStats > 0) { - // Due to passing a 'this' pointer, the current device object shouldn't be relocated, while the worker is - // active. - m_StatisticsUpdateWorker = std::unique_ptr(new StatisticsUpdateWorker( - *this, std::move(onStatsUpdate), onStatsUpdateUserCookie, intervalInSecondsToUpdateStats * 1000)); + StatisticsUpdateContext statsContext; + statsContext.cbOnStatsUpdate = std::move(onStatsUpdate); + statsContext.cbOnStatsUpdateUserCookie = onStatsUpdateUserCookie; + statsContext.updateIntervalMs = intervalInSecondsToUpdateStats * 1000; // Convert seconds to milliseconds + + m_StatsThread = std::thread(statsThreadMain, std::ref(m_StopThread), std::ref(m_PcapDescriptor), + std::move(statsContext)); PCPP_LOG_DEBUG("Successfully created stats thread for device '" << m_InterfaceDetails.name << "'."); } @@ -939,11 +909,10 @@ namespace pcpp } PCPP_LOG_DEBUG("Capture thread stopped for device '" << m_InterfaceDetails.name << "'"); - if (m_StatisticsUpdateWorker != nullptr) + if (m_StatsThread.joinable()) { PCPP_LOG_DEBUG("Stopping stats thread, waiting for it to join..."); - m_StatisticsUpdateWorker->stopWorker(); - m_StatisticsUpdateWorker.reset(); + m_StatsThread.join(); PCPP_LOG_DEBUG("Stats thread stopped for device '" << m_InterfaceDetails.name << "'"); } From 35542a02a201fb8fb05292a6e39d708a4751f76f Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 31 May 2025 23:59:13 +0300 Subject: [PATCH 7/8] Added nullptr defaults to contexts. --- Pcap++/src/PcapLiveDevice.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index a7bf49677..2c3533184 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -305,22 +305,22 @@ namespace pcpp struct CaptureContext { - PcapLiveDevice* device; + PcapLiveDevice* device = nullptr; OnPacketArrivesCallback callback; - void* userCookie; + void* userCookie = nullptr; }; struct AccumulatorCaptureContext { - PcapLiveDevice* device; - RawPacketVector* capturedPackets; + PcapLiveDevice* device = nullptr; + RawPacketVector* capturedPackets = nullptr; }; struct CaptureContextST { - PcapLiveDevice* device; + PcapLiveDevice* device = nullptr; OnPacketArrivesStopBlocking callback; - void* userCookie; + void* userCookie = nullptr; bool requestStop = false; }; From 448388502eccc820959d5be240535254e15afe2d Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sun, 1 Jun 2025 14:32:42 +0300 Subject: [PATCH 8/8] Added try catch blocks to catch exceptions bubbling out of dispatch callbacks. --- Pcap++/src/PcapLiveDevice.cpp | 72 +++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 2c3533184..ef306081c 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -296,8 +296,21 @@ namespace pcpp auto sleepDuration = std::chrono::milliseconds(context.updateIntervalMs); while (!stopFlag.load()) { - pcapDescriptor.getStatistics(stats); - context.cbOnStatsUpdate(stats, context.cbOnStatsUpdateUserCookie); + try + { + pcapDescriptor.getStatistics(stats); + context.cbOnStatsUpdate(stats, context.cbOnStatsUpdateUserCookie); + } + catch (const std::exception& ex) + { + PCPP_LOG_ERROR("Exception occurred while invoking statistics update callback: " << ex.what()); + break; + } + catch (...) + { + PCPP_LOG_ERROR("Unknown exception occurred while invoking statistics update callback"); + break; + } std::this_thread::sleep_for(sleepDuration); } PCPP_LOG_DEBUG("Ended statistics thread"); @@ -336,8 +349,19 @@ namespace pcpp return; } - RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); - context->callback(&rawPacket, context->device, context->userCookie); + try + { + RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); + context->callback(&rawPacket, context->device, context->userCookie); + } + catch (const std::exception& ex) + { + PCPP_LOG_ERROR("Exception occurred while invoking packet arrival callback: " << ex.what()); + } + catch (...) + { + PCPP_LOG_ERROR("Unknown exception occurred while invoking packet arrival callback"); + } } void onPacketArrivesAccumulator(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) @@ -349,11 +373,22 @@ namespace pcpp return; } - uint8_t* packetData = new uint8_t[pkthdr->caplen]; - std::memcpy(packetData, packet, pkthdr->caplen); - auto rawPacket = std::make_unique(packetData, pkthdr->caplen, pkthdr->ts, true, - context->device->getLinkType()); - context->capturedPackets->pushBack(std::move(rawPacket)); + try + { + uint8_t* packetData = new uint8_t[pkthdr->caplen]; + std::memcpy(packetData, packet, pkthdr->caplen); + auto rawPacket = std::make_unique(packetData, pkthdr->caplen, pkthdr->ts, true, + context->device->getLinkType()); + context->capturedPackets->pushBack(std::move(rawPacket)); + } + catch (const std::exception& ex) + { + PCPP_LOG_ERROR("Exception occurred while invoking packet arrival callback: " << ex.what()); + } + catch (...) + { + PCPP_LOG_ERROR("Unknown exception occurred while invoking packet arrival callback"); + } } void onPacketArrivesCallbackWithCancellation(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* packet) @@ -375,10 +410,23 @@ namespace pcpp RawPacket rawPacket(packet, pkthdr->caplen, pkthdr->ts, false, context->device->getLinkType()); - if (context->callback(&rawPacket, context->device, context->userCookie)) + try + { + if (context->callback(&rawPacket, context->device, context->userCookie)) + { + // If the callback returns true, it means that the user wants to stop the capture + context->requestStop = true; + } + } + catch (const std::exception& ex) + { + PCPP_LOG_ERROR("Exception occurred while invoking packet arrival callback: " << ex.what()); + context->requestStop = true; // Stop capture on exception + } + catch (...) { - // If the callback returns true, it means that the user wants to stop the capture - context->requestStop = true; + PCPP_LOG_ERROR("Unknown exception occurred while invoking packet arrival callback"); + context->requestStop = true; // Stop capture on unknown exception } }