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: (c sshnpd) allocate threaded memory outside of the thread #1171

Merged
merged 4 commits into from
Jul 3, 2024
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
2 changes: 1 addition & 1 deletion packages/c/cmake/atsdk.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ if(NOT atsdk_FOUND)
FetchContent_Declare(
atsdk
GIT_REPOSITORY https://github.com/atsign-foundation/at_c.git
GIT_TAG 504c299c48efdf7c9e6ecf9f7969ae9bedf495c7
GIT_TAG d8afd18535259b389344bc9d6392e213e7243650
)
FetchContent_MakeAvailable(atsdk)
install(TARGETS atclient atchops atlogger)
Expand Down
2 changes: 2 additions & 0 deletions packages/c/sshnpd/include/sshnpd/background_jobs.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ struct refresh_device_entry_params {
const char *payload;
const char *username;
volatile sig_atomic_t *should_run;
atclient_atkey *infokeys;
atclient_atkey *usernamekeys;
};

/**
Expand Down
32 changes: 3 additions & 29 deletions packages/c/sshnpd/src/background_jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@
void *refresh_device_entry(void *void_refresh_device_entry_params) {
struct refresh_device_entry_params *params = void_refresh_device_entry_params;

// Buffer for the atkeys
size_t num_managers = params->params->manager_list_len;
size_t num_username_keys = params->params->hide ? 0 : num_managers;

atclient_atkey infokeys[num_managers];
atclient_atkey usernamekeys[num_username_keys];
const size_t num_managers = params->params->manager_list_len;
atclient_atkey *infokeys = params->infokeys;
atclient_atkey *usernamekeys = params->usernamekeys;

// Buffer for the base portion of each atkey
size_t infokey_base_len = strlen(params->params->device) + strlen(params->params->atsign) +
Expand Down Expand Up @@ -55,7 +52,6 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) {
int index;
for (index = 0; index < num_managers; index++) {
// device_info
atclient_atkey_init(infokeys + index);
size_t buffer_len = strlen(params->params->manager_list[index]) + infokey_base_len;
char atkey_buffer[buffer_len];
// example: @client_atsign:device_info.device_name.sshnp@client_atsign
Expand All @@ -65,7 +61,6 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) {
if (ret != 0) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to create device_info atkey for %s\n",
params->params->manager_list[index]);
atclient_atkey_free(infokeys + index);
break;
}

Expand All @@ -76,17 +71,13 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) {
atclient_atkey_metadata_set_ccd(metadata, true);
atclient_atkey_metadata_set_ttl(metadata, (long)30 * 24 * 60 * 60 * 1000); // 30 days in ms

// username
atclient_atkey_init(usernamekeys + index);
buffer_len = strlen(params->params->manager_list[index]) + usernamekey_base_len;
// example: @client_atsign:device_info.device_name.sshnp@client_atsign
snprintf(atkey_buffer, buffer_len, "%s%s", params->params->manager_list[index], username_key_base);
ret = atclient_atkey_from_string(usernamekeys + index, atkey_buffer, buffer_len);
if (ret != 0) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to create username atkey for %s\n",
params->params->manager_list[index]);
atclient_atkey_free(infokeys + index);
atclient_atkey_free(usernamekeys + index);
break;
}

Expand All @@ -100,27 +91,19 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) {
if (ret != 0) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to delete username atkey for %s\n",
params->params->manager_list[index]);
atclient_atkey_free(infokeys + index);
atclient_atkey_free(usernamekeys + index);
break;
}
} else {
ret = atclient_put(params->atclient, usernamekeys + index, params->username, strlen(params->username), NULL);
if (ret != 0) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to put username atkey for %s\n",
params->params->manager_list[index]);
atclient_atkey_free(infokeys + index);
atclient_atkey_free(usernamekeys + index);
break;
}
}
}

if (ret != 0) {
for (int i = 0; i < index; i++) {
atclient_atkey_free(infokeys + i);
atclient_atkey_free(usernamekeys + i);
}
*params->should_run = 0;
}

Expand All @@ -135,10 +118,6 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) {
pthread_exit(NULL);
}

for (int i = 0; i < num_managers; i++) {
atclient_atkey_free(usernamekeys + i);
}

// Build each atkey
int interval_seconds = 60 * 60; // once an hour
int counter = 0;
Expand Down Expand Up @@ -190,10 +169,5 @@ void *refresh_device_entry(void *void_refresh_device_entry_params) {
sleep(1);
}

// Clean up upon exit
for (int i = 0; i < num_managers; i++) {
atclient_atkey_free(infokeys + i); // automatically cleans up metadata as well
}

pthread_exit(NULL);
}
29 changes: 28 additions & 1 deletion packages/c/sshnpd/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,27 @@ int main(int argc, char **argv) {

// 9. Start the device refresh loop - if hide is off
pthread_t refresh_tid;
atclient_atkey *infokeys = malloc(sizeof(atclient_atkey) * params.manager_list_len);
if (infokeys == NULL) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory for infokeys\n");
exit_res = 1;
goto cancel_atclient;
}

atclient_atkey *usernamekeys = malloc(sizeof(atclient_atkey) * params.manager_list_len);
if (usernamekeys == NULL) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to allocate memory for usernamekeys\n");
exit_res = 1;
goto clean_atkeys;
}

for(int i = 0; i < params.manager_list_len; i++) {
atclient_atkey_init(infokeys + i);
atclient_atkey_init(usernamekeys + i);
}

struct refresh_device_entry_params refresh_params = {&worker, &atclient_lock, &params,
ping_response, username, &should_run};
ping_response, username, &should_run, infokeys, usernamekeys};
res = pthread_create(&refresh_tid, NULL, refresh_device_entry, (void *)&refresh_params);
if (res != 0) {
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "Failed to start refresh device entry thread\n");
Expand Down Expand Up @@ -308,6 +327,14 @@ int main(int argc, char **argv) {
main_loop();
atlogger_log(LOGGER_TAG, ATLOGGER_LOGGING_LEVEL_INFO, "Exited main loop\n");

for(int i = 0; i < params.manager_list_len; i++) {
atclient_atkey_free(infokeys + i);
atclient_atkey_free(usernamekeys + i);
}

free(infokeys);
XavierChanth marked this conversation as resolved.
Show resolved Hide resolved
free(usernamekeys);

close_authkeys:
fclose(authkeys_file);
free(authkeys_filename);
Expand Down