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

Rate limits event loop #467

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8ebd018
First addition of rate-limit stuff. Untested.
Metalhead33 Oct 22, 2024
5610674
Fixed one typoo.
Metalhead33 Oct 22, 2024
2648337
Extracted the header extraction into a separate function, as suggeste…
Metalhead33 Oct 22, 2024
496f220
Changed minimum limit of delay from 30 to 65: at least one 16th of a …
Metalhead33 Oct 22, 2024
af3a8e8
Added default values, just in case.
Metalhead33 Nov 28, 2024
e70e799
Slowed down even further, just in case.
Metalhead33 Nov 28, 2024
3b2293d
Update libdiscord.c
Metalhead33 Nov 28, 2024
0e451b1
At this point, I think this might be necessary to prevent tempbans. N…
Metalhead33 Dec 20, 2024
31262aa
Merge branch 'EionRobb:master' into RateLimits
Metalhead33 Dec 20, 2024
11e8940
Conflict resolution.
Metalhead33 Dec 20, 2024
1ea7cc7
Typo fix.
Metalhead33 Dec 20, 2024
d25bfd0
Attempt to implement some sort of event loop for rate-limiting.
Metalhead33 Dec 20, 2024
a15a685
Added a token bucket of sorts.
Metalhead33 Jan 10, 2025
fede650
Added a check for the thread.
Metalhead33 Jan 10, 2025
4c8ae5c
Following the advice of CodeRabbit.
Metalhead33 Jan 10, 2025
2f249c4
Added yet another mutex to prevent a crash, but it only ended up free…
Metalhead33 Jan 10, 2025
adf5521
Replaced mutexes with recursive mutexes to prevent deadlock.
Metalhead33 Jan 10, 2025
e7860bc
Update ratelimiter.cpp
Metalhead33 Jan 10, 2025
356960b
Update ratelimiter.cpp
Metalhead33 Jan 10, 2025
8d25616
Added check for token acquisition timeout.
Metalhead33 Jan 10, 2025
e9899c7
Reintegrate with g_timeout_add
Metalhead33 Jan 10, 2025
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
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ PLUGIN_VERSION ?= 0.9.$(shell date +%Y.%m.%d)
endif

CFLAGS ?= -O2 -g -pipe -Wall

CPPFLAGS ?= -O2 -g -pipe -Wall
CFLAGS += -std=c99 -DDISCORD_PLUGIN_VERSION='"$(PLUGIN_VERSION)"' -DMARKDOWN_PIDGIN
CPPFLAGS+= -std=c++20
Comment on lines +30 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add essential compiler flags to CPPFLAGS

The C++ flags are missing important options needed for shared library compilation:

-CPPFLAGS ?= -O2 -g -pipe -Wall
+CPPFLAGS ?= -O2 -g -pipe -Wall -Wextra -Werror -fPIC

Also consider adding:

  • -Wconversion for implicit conversion warnings
  • -Wshadow for variable shadowing warnings
  • -Wno-unused-parameter to match C flags
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CPPFLAGS ?= -O2 -g -pipe -Wall
CFLAGS += -std=c99 -DDISCORD_PLUGIN_VERSION='"$(PLUGIN_VERSION)"' -DMARKDOWN_PIDGIN
CPPFLAGS+= -std=c++20
CPPFLAGS ?= -O2 -g -pipe -Wall -Wextra -Werror -fPIC
CFLAGS += -std=c99 -DDISCORD_PLUGIN_VERSION='"$(PLUGIN_VERSION)"' -DMARKDOWN_PIDGIN
CPPFLAGS+= -std=c++20


# Comment out to disable localisation
CFLAGS += -DENABLE_NLS
Expand Down Expand Up @@ -59,8 +60,10 @@ else
INCLUDES = -I/opt/local/include -lz $(OS)

CC = gcc
CXX = g++
else
CC ?= gcc
CXX = g++
LDFLAGS ?= -Wl,-z,relro
endif

Expand Down Expand Up @@ -107,7 +110,7 @@ CFLAGS += -DLOCALEDIR=\"$(LOCALEDIR)\"

C_FILES := markdown.c
PURPLE_COMPAT_FILES := purple2compat/*.c
PURPLE_C_FILES := libdiscord.c $(C_FILES)
PURPLE_C_FILES := ratelimiter.cpp libdiscord.c $(C_FILES)

.PHONY: all build install FAILNOPURPLE clean build-icons install-icons build-locales install-locales %-locale-install

Expand Down
63 changes: 61 additions & 2 deletions libdiscord.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "purple_compat.h"

#include "markdown.h"
#include "ratelimiter.h"

// Prevent segfault in libpurple ssl plugins
#define purple_ssl_read(a, b, c) ((a) && (a)->private_data ? purple_ssl_read((a), (b), (c)) : 0)
Expand Down Expand Up @@ -104,6 +105,28 @@ static GRegex *natural_mention_regex = NULL;
static GRegex *discord_mention_regex = NULL;
static GRegex *discord_spaced_mention_regex = NULL;

/*
xRateAllowedPerSecond = (int)( (double)xrateRemaining / (double)xrateResetAfter );
xRateAllowedRemaining = xRateAllowedPerSecond;
xRateDelayPerRequest = (int)((1.0 / (double)xRateAllowedPerSecond) * 1000.0);
*/

const int init_xrateLimit=40;
const int init_xrateRemaining=10;
const double init_xrateReset=55;
const double init_xrateResetAfter=1;
const int init_xRateAllowedPerSecond=15;
const int init_xRateDelayPerRequest=(int)((1.0 / (double)init_xRateAllowedPerSecond) * 1000.0);
const int init_xRateAllowedRemaining=10;

static int xrateLimit=init_xrateLimit;
static int xrateRemaining=init_xrateRemaining;
static double xrateReset=init_xrateReset;
static double xrateResetAfter=init_xrateResetAfter;
static int xRateAllowedPerSecond=init_xRateAllowedPerSecond;
static int xRateDelayPerRequest=init_xRateDelayPerRequest;
static int xRateAllowedRemaining=init_xRateAllowedRemaining;

typedef enum {
OP_DISPATCH = 0,
OP_HEARTBEAT = 1,
Expand Down Expand Up @@ -1449,6 +1472,39 @@ discord_cookies_to_string(DiscordAccount *ya)
}

static void discord_fetch_url_with_method_delay(DiscordAccount *da, const gchar *method, const gchar *url, const gchar *postdata, DiscordProxyCallbackFunc callback, gpointer user_data, guint delay);
static void UpdateRateLimits(const gchar *xrateLimitS, const gchar *xrateRemainingS, const gchar *xRateReset, const gchar *xRateResetAfter)
{
if(!xrateLimitS || !xrateRemainingS || !xRateReset || !xRateResetAfter) return;
xrateLimit=atoi(xrateLimitS);
purple_debug_info("discord", "X-RateLimit-Limit: %s\n", xrateLimitS);
xrateRemaining=atoi(xrateRemainingS);
purple_debug_info("discord", "X-RateLimit-Remaining: %s\n", xrateRemainingS);
xrateReset=atof(xRateReset);
purple_debug_info("discord", "X-RateLimit-Reset: %s\n", xRateReset);
xrateResetAfter=atof(xRateResetAfter) * 4.0;
purple_debug_info("discord", "X-RateLimit-Reset-After: %s\n", xRateResetAfter);
if (xrateResetAfter > 0) {
xRateAllowedPerSecond = (int)( (double)xrateRemaining / (double)xrateResetAfter );
xRateAllowedRemaining = xRateAllowedPerSecond;
xRateDelayPerRequest = xRateAllowedPerSecond > 0 ?
(int)((1.0 / (double)xRateAllowedPerSecond) * 1000.0) : 1000;
purple_debug_info("discord", "Rate limits calculated: %d requests/sec, %d ms delay\n",
xRateAllowedPerSecond, xRateDelayPerRequest);
initialize_rate_limiter(xRateDelayPerRequest);
} else {
purple_debug_warning("discord", "Invalid rate limit reset value\n");
xRateAllowedPerSecond = 1;
xRateAllowedRemaining = 1;
xRateDelayPerRequest = 1000;
}
}
Metalhead33 marked this conversation as resolved.
Show resolved Hide resolved
static void parse_rate_limit_headers(PurpleHttpResponse *response) {
const gchar *xrateLimitS = purple_http_response_get_header(response,"X-RateLimit-Limit");
const gchar *xrateRemainingS = purple_http_response_get_header(response,"X-RateLimit-Remaining");
const gchar *xRateReset = purple_http_response_get_header(response,"X-RateLimit-Reset");
const gchar *xRateResetAfter = purple_http_response_get_header(response,"X-RateLimit-Reset-After");
UpdateRateLimits(xrateLimitS, xrateRemainingS, xRateReset, xRateResetAfter);
}

static void
discord_response_callback(PurpleHttpConnection *http_conn,
Expand All @@ -1457,6 +1513,7 @@ discord_response_callback(PurpleHttpConnection *http_conn,
gsize len;
const gchar *url_text = purple_http_response_get_data(response, &len);
const gchar *error_message = purple_http_response_get_error(response);
parse_rate_limit_headers(response);
const gchar *body;
gsize body_len;
DiscordProxyConnection *conn = user_data;
Expand Down Expand Up @@ -1635,7 +1692,7 @@ discord_fetch_url_with_method_delay(DiscordAccount *da, const gchar *method, con
request->url = g_strdup(url);
request->contents = postdata ? g_strdup(postdata) : NULL;

purple_timeout_add(delay + 30, discord_fetch_url_with_method_delay_cb, request);
rlimited_timeout_add(delay + MAX(65,xRateDelayPerRequest), discord_fetch_url_with_method_delay_cb, request);
}

static void
Expand Down Expand Up @@ -6277,7 +6334,7 @@ discord_socket_delay_write_data(DiscordAccount *ya, guchar *data, gsize data_len
info->type = type;

// Set timer for when to check the bucket again. Could probably make this more intelligent.
purple_timeout_add(1000, discord_socket_write_data_delay_cb, info);
rlimited_timeout_add(1000, discord_socket_write_data_delay_cb, info);
}

static void
Expand Down Expand Up @@ -10615,6 +10672,7 @@ PURPLE_DEFINE_TYPE_EXTENDED(
static gboolean
libpurple3_plugin_load(PurplePlugin *plugin, GError **error)
{
initialize_rate_limiter(83);
discord_protocol_register_type(plugin);
discord_protocol = purple_protocols_add(DISCORD_TYPE_PROTOCOL, error);

Expand All @@ -10628,6 +10686,7 @@ libpurple3_plugin_load(PurplePlugin *plugin, GError **error)
static gboolean
libpurple3_plugin_unload(PurplePlugin *plugin, GError **error)
{
stop_rate_limiter();
if (!plugin_unload(plugin, error)) {
return FALSE;
}
Expand Down
Loading