SolidWallOfCode commented on code in PR #10688:
URL: https://github.com/apache/trafficserver/pull/10688#discussion_r1383839164
##########
src/iocore/net/ConnectionTracker.cc:
##########
@@ -222,25 +287,46 @@ ConnectionTracker::Group::should_alert(std::time_t *lat)
return zret;
}
+void
+ConnectionTracker::Group::release()
+{
+ if (_count >= 0) {
+ --_count;
+ if (_count == 0) {
+ TableSingleton &table = _direction == DirectionType::INBOUND ?
_inbound_table : _outbound_table;
+ std::lock_guard<std::mutex> lock(table._mutex); // Table lock
+ if (_count > 0) {
+ // Someone else grabbed the Group between our last check and taking the
+ // lock.
+ return;
+ }
+ table._table.erase(_key);
+ }
+ } else {
+ // A bit dubious, as there's no guarantee it's still negative, but even
that would be interesting to know.
+ Error("Number of tracked connections should be greater than or equal to
zero: %u", _count.load());
+ }
+}
+
std::time_t
ConnectionTracker::Group::get_last_alert_epoch_time() const
{
return Clock::to_time_t(TimePoint{TimePoint::duration{Ticker{_last_alert}}});
}
void
-ConnectionTracker::get(std::vector<Group const *> &groups)
+ConnectionTracker::get_outbound_groups(std::vector<std::shared_ptr<Group
const>> &groups)
{
- std::lock_guard<std::mutex> lock(_imp._mutex); // TABLE LOCK
+ std::lock_guard<std::mutex> lock(_outbound_table._mutex); // TABLE LOCK
groups.resize(0);
Review Comment:
I'd be tempted to move these outside the lock. Worse case is the table grows
and an allocation is required inside the lock, but this code has an allocation
inside the lock every time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
