Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use mutex instead of atomic for large types to avoid linking issues. #306

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 16 additions & 27 deletions src/torrent/tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ Tracker::Tracker(TrackerList* parent, const std::string& url, int flags) :
m_url(url),
m_request_time_last(torrent::cachedTime.seconds())
{
// TODO: Not needed when EVENT_NONE is default.
auto tracker_state = TrackerState{};
tracker_state.m_latest_event = EVENT_NONE;
m_state.store(tracker_state);
m_state.m_latest_event = EVENT_NONE;
}

void
Expand Down Expand Up @@ -78,48 +75,40 @@ Tracker::inc_request_counter() {

void
Tracker::clear_intervals() {
auto state = m_state.load();
std::lock_guard<std::mutex> lock(m_state_mutex);

state.m_normal_interval = 0;
state.m_min_interval = 0;

m_state.store(state);
m_state.m_normal_interval = 0;
m_state.m_min_interval = 0;
}

void
Tracker::clear_stats() {
auto state = m_state.load();

state.m_latest_new_peers = 0;
state.m_latest_sum_peers = 0;
state.m_success_counter = 0;
state.m_failed_counter = 0;
state.m_scrape_counter = 0;
std::lock_guard<std::mutex> lock(m_state_mutex);

m_state.store(state);
m_state.m_latest_new_peers = 0;
m_state.m_latest_sum_peers = 0;
m_state.m_success_counter = 0;
m_state.m_failed_counter = 0;
m_state.m_scrape_counter = 0;
}

void
Tracker::set_latest_event(int v) {
auto state = m_state.load();

state.m_latest_event = v;

m_state.store(state);
std::lock_guard<std::mutex> lock(m_state_mutex);
m_state.m_latest_event = v;
}

void
Tracker::update_tracker_id(const std::string& id) {
if (id.empty())
return;

if (tracker_id() == id)
return;
std::lock_guard<std::mutex> lock(m_state_mutex);

auto new_id = std::array<char,64>{};
std::copy_n(id.begin(), std::min(id.size(), size_t(63)), new_id.begin());
if (m_tracker_id == id)
return;

m_tracker_id.store(new_id);
m_tracker_id = id;
}

}
38 changes: 30 additions & 8 deletions src/torrent/tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <array>
#include <atomic>
#include <cinttypes>
#include <mutex>
#include <string>
#include <torrent/common.h>
#include <torrent/tracker/tracker_state.h>
Expand Down Expand Up @@ -54,12 +55,12 @@ class LIBTORRENT_EXPORT Tracker {

bool is_enabled() const { return (m_flags & flag_enabled); }
bool is_extra_tracker() const { return (m_flags & flag_extra_tracker); }
bool is_in_use() const { return is_enabled() && m_state.load().success_counter() != 0; }
bool is_in_use() const { return is_enabled() && state().success_counter() != 0; }

bool can_scrape() const { return (m_flags & flag_can_scrape); }

virtual bool is_busy() const = 0;
bool is_busy_not_scrape() const { return m_state.load().latest_event() != EVENT_SCRAPE && is_busy(); }
bool is_busy_not_scrape() const { return state().latest_event() != EVENT_SCRAPE && is_busy(); }
virtual bool is_usable() const { return is_enabled(); }

bool can_request_state() const;
Expand All @@ -75,9 +76,8 @@ class LIBTORRENT_EXPORT Tracker {
const std::string& url() const { return m_url; }
void set_url(const std::string& url) { m_url = url; }

std::string tracker_id() const { return std::string(m_tracker_id.load().data()); }

TrackerState state() const { return m_state.load(); }
std::string tracker_id() const;
TrackerState state() const;;

virtual void get_status(char* buffer, [[maybe_unused]] int length) { buffer[0] = 0; }

Expand All @@ -104,29 +104,51 @@ class LIBTORRENT_EXPORT Tracker {
void clear_stats();

void set_latest_event(int v);
void set_state(const TrackerState& state);
void update_tracker_id(const std::string& id);

int m_flags;

TrackerList* m_parent;
uint32_t m_group{0};

std::string m_url;
std::atomic<std::array<char,64>> m_tracker_id{{}};
std::string m_url;

// Timing of the last request, and a counter for how many requests
// there's been in the recent past.
uint32_t m_request_time_last;
uint32_t m_request_counter{0};

std::atomic<TrackerState> m_state;
private:
// Only the tracker thread should change state.
mutable std::mutex m_state_mutex;
TrackerState m_state;
std::string m_tracker_id;
};

inline bool
Tracker::can_request_state() const {
return !(is_busy() && state().latest_event() != EVENT_SCRAPE) && is_usable();
}

inline std::string
Tracker::tracker_id() const {
std::lock_guard<std::mutex> lock(m_state_mutex);
return m_tracker_id;
}

inline TrackerState
Tracker::state() const {
std::lock_guard<std::mutex> lock(m_state_mutex);
return m_state;
}

inline void
Tracker::set_state(const TrackerState& state) {
std::lock_guard<std::mutex> lock(m_state_mutex);
m_state = state;
}

}

#endif
3 changes: 3 additions & 0 deletions src/torrent/tracker/tracker_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <algorithm>
#include <torrent/common.h>

class TrackerTest;

namespace torrent {

class TrackerDht;
Expand All @@ -17,6 +19,7 @@ class TrackerState {
friend class TrackerDht;
friend class TrackerHttp;
friend class TrackerList;
friend class ::TrackerTest;
friend class TrackerUdp;

static constexpr int default_min_interval = 600;
Expand Down
6 changes: 3 additions & 3 deletions src/torrent/tracker_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ TrackerList::receive_success(Tracker* tracker, AddressList* l) {
tracker_state.m_latest_sum_peers = l->size();
tracker_state.m_latest_new_peers = m_slot_success(tracker, l);

tracker->m_state.store(tracker_state);
tracker->set_state(tracker_state);
}

void
Expand All @@ -318,7 +318,7 @@ TrackerList::receive_failed(Tracker* tracker, const std::string& msg) {
tracker_state.m_failed_time_last = cachedTime.seconds();
tracker_state.m_failed_counter++;

tracker->m_state.store(tracker_state);
tracker->set_state(tracker_state);

m_slot_failed(tracker, msg);
}
Expand All @@ -337,7 +337,7 @@ TrackerList::receive_scrape_success(Tracker* tracker) {
tracker_state.m_scrape_time_last = cachedTime.seconds();
tracker_state.m_scrape_counter++;

tracker->m_state.store(tracker_state);
tracker->set_state(tracker_state);

if (m_slot_scrape_success)
m_slot_scrape_success(tracker);
Expand Down
2 changes: 1 addition & 1 deletion src/tracker/tracker_dht.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ TrackerDht::send_state(int new_state) {
auto tracker_state = state();
tracker_state.set_normal_interval(20 * 60);
tracker_state.set_min_interval(0);
m_state.store(tracker_state);
set_state(tracker_state);
}

void
Expand Down
17 changes: 9 additions & 8 deletions src/tracker/tracker_http.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ TrackerHttp::send_state(int new_state) {

char localId[61];

DownloadInfo* info = m_parent->info();
auto info = m_parent->info();
auto tracker_id = this->tracker_id();

request_prefix(&s, m_url);

Expand All @@ -101,8 +102,8 @@ TrackerHttp::send_state(int new_state) {
if (m_parent->key())
s << "&key=" << std::hex << std::setw(8) << std::setfill('0') << m_parent->key() << std::dec;

if (!m_tracker_id.load().empty())
s << "&trackerid=" << rak::copy_escape_html(tracker_id());
if (!tracker_id.empty())
s << "&trackerid=" << rak::copy_escape_html(tracker_id);

const rak::socket_address* localAddress = rak::socket_address::cast_from(manager->connection_manager()->local_address());

Expand Down Expand Up @@ -324,12 +325,12 @@ TrackerHttp::process_failure(const Object& object) {
if (object.has_key_value("downloaded"))
tracker_state.m_scrape_downloaded = std::max<int64_t>(object.get_key_value("downloaded"), 0);

m_state.store(tracker_state);
set_state(tracker_state);
}

void
TrackerHttp::process_success(const Object& object) {
auto tracker_state = m_state.load();
auto tracker_state = state();

if (object.has_key_value("interval"))
tracker_state.set_normal_interval(object.get_key_value("interval"));
Expand All @@ -353,7 +354,7 @@ TrackerHttp::process_success(const Object& object) {
if (object.has_key_value("downloaded"))
tracker_state.m_scrape_downloaded = std::max<int64_t>(object.get_key_value("downloaded"), 0);

m_state.store(tracker_state);
set_state(tracker_state);

if (!object.has_key("peers") && !object.has_key("peers6"))
return receive_failed("No peers returned");
Expand Down Expand Up @@ -395,7 +396,7 @@ TrackerHttp::process_scrape(const Object& object) {

const Object& stats = files.get_key(m_parent->info()->hash().str());

auto tracker_state = m_state.load();
auto tracker_state = state();

if (stats.has_key_value("complete"))
tracker_state.m_scrape_complete = std::max<int64_t>(stats.get_key_value("complete"), 0);
Expand All @@ -406,7 +407,7 @@ TrackerHttp::process_scrape(const Object& object) {
if (stats.has_key_value("downloaded"))
tracker_state.m_scrape_downloaded = std::max<int64_t>(stats.get_key_value("downloaded"), 0);

m_state.store(tracker_state);
set_state(tracker_state);

LT_LOG_TRACKER(INFO, "Tracker scrape for %u torrents: complete:%u incomplete:%u downloaded:%u.",
files.as_map().size(), tracker_state.m_scrape_complete, tracker_state.m_scrape_incomplete, tracker_state.m_scrape_downloaded);
Expand Down
2 changes: 1 addition & 1 deletion src/tracker/tracker_udp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ TrackerUdp::process_announce_output() {
tracker_state.m_scrape_complete = m_readBuffer->read_32(); // seeders
tracker_state.m_scrape_time_last = rak::timer::current().seconds();

m_state.store(tracker_state);
set_state(tracker_state);

AddressList l;

Expand Down
44 changes: 0 additions & 44 deletions test/torrent/net/Makefile.am

This file was deleted.

Loading
Loading