Skip to content

Commit 0a6241a

Browse files
authored
Move PcapLiveDevice statistics machinery entirely inside the .cpp file. (#1966)
* Move PcapLiveDevice statistics entirely inside the .cpp file. * Update debug logs.
1 parent 96484fb commit 0a6241a

File tree

2 files changed

+47
-102
lines changed

2 files changed

+47
-102
lines changed

Pcap++/header/PcapLiveDevice.h

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -82,44 +82,6 @@ namespace pcpp
8282
bool isLoopback;
8383
};
8484

85-
/// @brief A worker thread that periodically calls the provided callback with updated statistics.
86-
class StatisticsUpdateWorker
87-
{
88-
public:
89-
/// @brief Constructs and starts a worker thread that periodically calls the provided callback with updated
90-
/// statistics.
91-
/// @param pcapDevice A pointer to the PcapLiveDevice instance to be monitored.
92-
/// @param onStatsUpdateCallback A callback function to be called with updated statistics.
93-
/// @param onStatsUpdateUserCookie A user-defined pointer that is passed to the callback function.
94-
/// @param updateIntervalMs The interval in milliseconds between each callback invocation.
95-
StatisticsUpdateWorker(PcapLiveDevice const& pcapDevice, OnStatsUpdateCallback onStatsUpdateCallback,
96-
void* onStatsUpdateUserCookie = nullptr, unsigned int updateIntervalMs = 1000);
97-
98-
/// @brief Stops the worker thread.
99-
void stopWorker();
100-
101-
private:
102-
struct ThreadData
103-
{
104-
PcapLiveDevice const* pcapDevice = nullptr;
105-
OnStatsUpdateCallback cbOnStatsUpdate;
106-
void* cbOnStatsUpdateUserCookie = nullptr;
107-
unsigned int updateIntervalMs = 1000; // Default update interval is 1 second
108-
};
109-
110-
struct SharedThreadData
111-
{
112-
std::atomic_bool stopRequested{ false };
113-
};
114-
115-
/// @brief Main function for the worker thread.
116-
/// @remarks This function is static to allow the worker class to be movable.
117-
static void workerMain(std::shared_ptr<SharedThreadData> sharedThreadData, ThreadData threadData);
118-
119-
std::shared_ptr<SharedThreadData> m_SharedThreadData;
120-
std::thread m_WorkerThread;
121-
};
122-
12385
bool m_DeviceOpened = false;
12486

12587
// This is a second descriptor for the same device. It is needed because of a bug
@@ -134,9 +96,7 @@ namespace pcpp
13496
MacAddress m_MacAddress;
13597
IPv4Address m_DefaultGateway;
13698
std::thread m_CaptureThread;
137-
138-
// TODO: Cpp17 Using std::optional might be better here
139-
std::unique_ptr<StatisticsUpdateWorker> m_StatisticsUpdateWorker;
99+
std::thread m_StatsThread;
140100

141101
// Should be set to true by the Caller for the Callee
142102
std::atomic<bool> m_StopThread;

Pcap++/src/PcapLiveDevice.cpp

Lines changed: 46 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -230,67 +230,49 @@ namespace pcpp
230230
}
231231
}
232232

233-
PcapLiveDevice::StatisticsUpdateWorker::StatisticsUpdateWorker(PcapLiveDevice const& pcapDevice,
234-
OnStatsUpdateCallback onStatsUpdateCallback,
235-
void* onStatsUpdateUserCookie,
236-
unsigned int updateIntervalMs)
237-
{
238-
// Setup thread data
239-
m_SharedThreadData = std::make_shared<SharedThreadData>();
240-
241-
ThreadData threadData;
242-
threadData.pcapDevice = &pcapDevice;
243-
threadData.cbOnStatsUpdate = onStatsUpdateCallback;
244-
threadData.cbOnStatsUpdateUserCookie = onStatsUpdateUserCookie;
245-
threadData.updateIntervalMs = updateIntervalMs;
246-
247-
// Start the thread
248-
m_WorkerThread = std::thread(&StatisticsUpdateWorker::workerMain, m_SharedThreadData, std::move(threadData));
249-
}
250-
251-
void PcapLiveDevice::StatisticsUpdateWorker::stopWorker()
252-
{
253-
m_SharedThreadData->stopRequested = true;
254-
if (m_WorkerThread.joinable())
255-
{
256-
m_WorkerThread.join();
257-
}
258-
}
259-
260-
void PcapLiveDevice::StatisticsUpdateWorker::workerMain(std::shared_ptr<SharedThreadData> sharedThreadData,
261-
ThreadData threadData)
233+
namespace
262234
{
263-
if (sharedThreadData == nullptr)
264-
{
265-
PCPP_LOG_ERROR("Shared thread data is null");
266-
return;
267-
}
268-
269-
if (threadData.pcapDevice == nullptr)
235+
struct StatisticsUpdateContext
270236
{
271-
PCPP_LOG_ERROR("Pcap device is null");
272-
return;
273-
}
237+
OnStatsUpdateCallback cbOnStatsUpdate;
238+
void* cbOnStatsUpdateUserCookie = nullptr;
239+
std::chrono::milliseconds updateInterval = std::chrono::seconds(1);
240+
};
274241

275-
if (threadData.cbOnStatsUpdate == nullptr)
242+
void statsThreadMain(std::atomic<bool>& stopFlag, internal::PcapHandle const& pcapDescriptor,
243+
StatisticsUpdateContext context)
276244
{
277-
PCPP_LOG_ERROR("Statistics Callback is null");
278-
return;
279-
}
245+
if (context.cbOnStatsUpdate == nullptr)
246+
{
247+
PCPP_LOG_ERROR("No callback provided for statistics updates");
248+
return;
249+
}
280250

281-
PCPP_LOG_DEBUG("Started statistics thread");
251+
PCPP_LOG_DEBUG("Begin periodic statistics update procedure");
282252

283-
PcapStats stats;
284-
auto sleepDuration = std::chrono::milliseconds(threadData.updateIntervalMs);
285-
while (!sharedThreadData->stopRequested)
286-
{
287-
threadData.pcapDevice->getStatistics(stats);
288-
threadData.cbOnStatsUpdate(stats, threadData.cbOnStatsUpdateUserCookie);
289-
std::this_thread::sleep_for(sleepDuration);
253+
IPcapDevice::PcapStats stats;
254+
while (!stopFlag.load())
255+
{
256+
try
257+
{
258+
pcapDescriptor.getStatistics(stats);
259+
context.cbOnStatsUpdate(stats, context.cbOnStatsUpdateUserCookie);
260+
}
261+
catch (const std::exception& ex)
262+
{
263+
PCPP_LOG_ERROR("Exception occurred while invoking statistics update callback: " << ex.what());
264+
break;
265+
}
266+
catch (...)
267+
{
268+
PCPP_LOG_ERROR("Unknown exception occurred while invoking statistics update callback");
269+
break;
270+
}
271+
std::this_thread::sleep_for(context.updateInterval);
272+
}
273+
PCPP_LOG_DEBUG("Ended periodic statistics update procedure");
290274
}
291-
292-
PCPP_LOG_DEBUG("Stopped statistics thread");
293-
}
275+
} // namespace
294276

295277
PcapLiveDevice::PcapLiveDevice(DeviceInterfaceDetails interfaceDetails, bool calculateMTU, bool calculateMacAddress,
296278
bool calculateDefaultGateway)
@@ -686,10 +668,14 @@ namespace pcpp
686668

687669
if (onStatsUpdate != nullptr && intervalInSecondsToUpdateStats > 0)
688670
{
689-
// Due to passing a 'this' pointer, the current device object shouldn't be relocated, while the worker is
690-
// active.
691-
m_StatisticsUpdateWorker = std::make_unique<StatisticsUpdateWorker>(
692-
*this, std::move(onStatsUpdate), onStatsUpdateUserCookie, intervalInSecondsToUpdateStats * 1000);
671+
StatisticsUpdateContext statsContext;
672+
673+
statsContext.cbOnStatsUpdate = std::move(onStatsUpdate);
674+
statsContext.cbOnStatsUpdateUserCookie = onStatsUpdateUserCookie;
675+
statsContext.updateInterval = std::chrono::seconds(intervalInSecondsToUpdateStats);
676+
677+
m_StatsThread = std::thread(statsThreadMain, std::ref(m_StopThread), std::ref(m_PcapDescriptor),
678+
std::move(statsContext));
693679

694680
PCPP_LOG_DEBUG("Successfully created stats thread for device '" << m_InterfaceDetails.name << "'.");
695681
}
@@ -890,11 +876,10 @@ namespace pcpp
890876
}
891877
PCPP_LOG_DEBUG("Capture thread stopped for device '" << m_InterfaceDetails.name << "'");
892878

893-
if (m_StatisticsUpdateWorker != nullptr)
879+
if (m_StatsThread.joinable())
894880
{
895881
PCPP_LOG_DEBUG("Stopping stats thread, waiting for it to join...");
896-
m_StatisticsUpdateWorker->stopWorker();
897-
m_StatisticsUpdateWorker.reset();
882+
m_StatsThread.join();
898883
PCPP_LOG_DEBUG("Stats thread stopped for device '" << m_InterfaceDetails.name << "'");
899884
}
900885

0 commit comments

Comments
 (0)