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

Fix socket reuse #2526

Merged
merged 1 commit into from
Oct 24, 2023
Merged
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
29 changes: 13 additions & 16 deletions src/curlEngine.d
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@ import std.stdio;

class CurlEngine {
HTTP http;
bool keepAlive;

this() {
http = HTTP();
}

void initialise(long dnsTimeout, long connectTimeout, long dataTimeout, long operationTimeout, int maxRedirects, bool httpsDebug, string userAgent, bool httpProtocol, long userRateLimit, long protocolVersion) {
void initialise(long dnsTimeout, long connectTimeout, long dataTimeout, long operationTimeout, int maxRedirects, bool httpsDebug, string userAgent, bool httpProtocol, long userRateLimit, long protocolVersion, bool keepAlive=false) {
// Setting this to false ensures that when we close the curl instance, any open sockets are closed - which we need to do when running
// multiple threads and API instances at the same time otherwise we run out of local files | sockets pretty quickly
this.keepAlive = keepAlive;

// Curl Timeout Handling

// libcurl dns_cache_timeout timeout
Expand Down Expand Up @@ -77,14 +82,6 @@ class CurlEngine {
// Ensure that TCP_NODELAY is set to 0 to ensure that TCP NAGLE is enabled
http.handle.set(CurlOption.tcp_nodelay,0);

// https://curl.se/libcurl/c/CURLOPT_FORBID_REUSE.html
// CURLOPT_FORBID_REUSE - make connection get closed at once after use
// Ensure that we ARE NOT reusing TCP sockets connections - setting to 0 ensures that we ARE reusing connections (we did this in v2.4.xx) to ensure connections remained open and usable
// Setting this to 1 ensures that when we close the curl instance, any open sockets are closed - which we need to do when running
// multiple threads and API instances at the same time otherwise we run out of local files | sockets pretty quickly
// The libcurl default is 1 - ensure we are configuring not to reuse connections and leave unused sockets open
http.handle.set(CurlOption.forbid_reuse,1);

if (httpsDebug) {
// Output what options we are using so that in the debug log this can be tracked
log.vdebug("http.dnsTimeout = ", dnsTimeout);
Expand All @@ -93,15 +90,15 @@ class CurlEngine {
log.vdebug("http.operationTimeout = ", operationTimeout);
log.vdebug("http.maxRedirects = ", maxRedirects);
log.vdebug("http.CurlOption.ipresolve = ", protocolVersion);
log.vdebug("http.header.Connection.keepAlive = ", keepAlive);
}
}

void setMethodPost() {
http.method = HTTP.Method.post;
}

void setMethodPatch() {
http.method = HTTP.Method.patch;

void connect(HTTP.Method method, const(char)[] url) {
if (!keepAlive)
http.addRequestHeader("Connection", "close");
http.method = method;
http.url = url;
}

void setDisableSSLVerifyPeer() {
Expand Down
25 changes: 9 additions & 16 deletions src/onedrive.d
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ class OneDriveApi {
}

// Initialise the OneDrive API class
bool initialise() {
bool initialise(bool keepAlive=false) {
// Initialise the curl engine
curlEngine = new CurlEngine();
curlEngine.initialise(appConfig.getValueLong("dns_timeout"), appConfig.getValueLong("connect_timeout"), appConfig.getValueLong("data_timeout"), appConfig.getValueLong("operation_timeout"), appConfig.defaultMaxRedirects, appConfig.getValueBool("debug_https"), appConfig.getValueString("user_agent"), appConfig.getValueBool("force_http_11"), appConfig.getValueLong("rate_limit"), appConfig.getValueLong("ip_protocol_version"));
curlEngine.initialise(appConfig.getValueLong("dns_timeout"), appConfig.getValueLong("connect_timeout"), appConfig.getValueLong("data_timeout"), appConfig.getValueLong("operation_timeout"), appConfig.defaultMaxRedirects, appConfig.getValueBool("debug_https"), appConfig.getValueString("user_agent"), appConfig.getValueBool("force_http_11"), appConfig.getValueLong("rate_limit"), appConfig.getValueLong("ip_protocol_version"), keepAlive);

// Authorised value to return
bool authorised = false;
Expand Down Expand Up @@ -758,8 +758,7 @@ class OneDriveApi {
}
}

curlEngine.http.method = HTTP.Method.put;
curlEngine.http.url = uploadUrl;
curlEngine.connect(HTTP.Method.put, uploadUrl);
curlEngine.http.addRequestHeader("Content-Range", contentRange);
curlEngine.http.onSend = data => file.rawRead(data).length;
// convert offsetSize to ulong
Expand Down Expand Up @@ -1230,8 +1229,7 @@ class OneDriveApi {

private void performDelete(const(char)[] url) {
scope(exit) curlEngine.http.clearRequestHeaders();
curlEngine.http.method = HTTP.Method.del;
curlEngine.http.url = url;
curlEngine.connect(HTTP.Method.del, url);
addAccessTokenHeader();
auto response = performHTTPOperation();
checkHttpResponseCode(response);
Expand Down Expand Up @@ -1268,8 +1266,7 @@ class OneDriveApi {
}
}

curlEngine.http.method = HTTP.Method.get;
curlEngine.http.url = url;
curlEngine.connect(HTTP.Method.get, url);
addAccessTokenHeader();

curlEngine.http.onReceive = (ubyte[] data) {
Expand Down Expand Up @@ -1392,8 +1389,7 @@ class OneDriveApi {
private JSONValue get(string url, bool skipToken = false) {
scope(exit) curlEngine.http.clearRequestHeaders();
log.vdebug("Request URL = ", url);
curlEngine.http.method = HTTP.Method.get;
curlEngine.http.url = url;
curlEngine.connect(HTTP.Method.get, url);
if (!skipToken) addAccessTokenHeader(); // HACK: requestUploadStatus
JSONValue response;
response = performHTTPOperation();
Expand All @@ -1418,8 +1414,7 @@ class OneDriveApi {

private auto patch(T)(const(char)[] url, const(T)[] patchData) {
scope(exit) curlEngine.http.clearRequestHeaders();
curlEngine.setMethodPatch();
curlEngine.http.url = url;
curlEngine.connect(HTTP.Method.patch, url);
addAccessTokenHeader();
auto response = perform(patchData);
checkHttpResponseCode(response);
Expand All @@ -1428,8 +1423,7 @@ class OneDriveApi {

private auto post(T)(string url, const(T)[] postData) {
scope(exit) curlEngine.http.clearRequestHeaders();
curlEngine.setMethodPost();
curlEngine.http.url = url;
curlEngine.connect(HTTP.Method.post, url);
addAccessTokenHeader();
auto response = perform(postData);
checkHttpResponseCode(response);
Expand Down Expand Up @@ -1649,8 +1643,7 @@ class OneDriveApi {
}
}

curlEngine.http.method = HTTP.Method.put;
curlEngine.http.url = url;
curlEngine.connect(HTTP.Method.put, url);
addAccessTokenHeader();
curlEngine.http.addRequestHeader("Content-Type", "application/octet-stream");
curlEngine.http.onSend = data => file.rawRead(data).length;
Expand Down
6 changes: 4 additions & 2 deletions src/sync.d
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class SyncEngine {
OneDriveApi oneDriveApiInstance;
ItemDatabase itemDB;
ClientSideFiltering selectiveSync;

// Array of directory databaseItem.id to skip while applying the changes.
// These are the 'parent path' id's that are being excluded, so if the parent id is in here, the child needs to be skipped as well
RedBlackTree!string skippedItems = redBlackTree!string();
Expand Down Expand Up @@ -716,9 +716,11 @@ class SyncEngine {
}

// Create a new API Instance for querying /delta and initialise it
// Reuse the socket to speed up
bool keepAlive = true;
OneDriveApi getDeltaQueryOneDriveApiInstance;
getDeltaQueryOneDriveApiInstance = new OneDriveApi(appConfig);
getDeltaQueryOneDriveApiInstance.initialise();
getDeltaQueryOneDriveApiInstance.initialise(keepAlive);

for (;;) {
responseBundleCount++;
Expand Down
3 changes: 1 addition & 2 deletions src/util.d
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,8 @@ bool testInternetReachability(ApplicationConfig appConfig) {

// Configure the remaining items required
// URL to use
curlEngine.http.url = "https://login.microsoftonline.com";
// HTTP connection test method
curlEngine.http.method = HTTP.Method.head;
curlEngine.connect(HTTP.Method.head, "https://login.microsoftonline.com");
// Attempt to contact the Microsoft Online Service
try {
log.vdebug("Attempting to contact Microsoft OneDrive Login Service");
Expand Down