Skip to content

Commit

Permalink
Fix curl reusing socket (#2526)
Browse files Browse the repository at this point in the history
* Fix curl reusing socket
  • Loading branch information
JC-comp authored Oct 24, 2023
1 parent 5dedd26 commit 1d29ca0
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 36 deletions.
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

0 comments on commit 1d29ca0

Please sign in to comment.