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.

Sign up for GitHub

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

Jump to bottom

Fix race condition when client set to not reconnect #626

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Fix race condition when client set to not reconnect #626

merged 1 commit into from
Oct 31, 2024

Conversation

euripedesrocha
Copy link
Collaborator

Fix: #412

@euripedesrocha euripedesrocha self-assigned this Aug 12, 2024
david-cermak
david-cermak approved these changes Aug 16, 2024
View reviewed changes
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

LGTM!

FWIW we already support destroying asynchronously, so the usecase described in #412 would not be necessary (still possible though), see:

esp-protocols/components/esp_websocket_client/include/esp_websocket_client.h

Lines 228 to 238 in e425a3c

/**
* @brief If this API called, WebSocket client will destroy and free all resources at the end of event loop.
*
* Notes:
* - After event loop finished, client handle would be dangling and should never be used
*
* @param[in] client The client
*
* @return esp_err_t
*/
esp_err_t esp_websocket_client_destroy_on_exit(esp_websocket_client_handle_t client);

github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 19, 2024
View reviewed changes
components/console_cmd_ping/console_getsetdnsserver.c Outdated
while ((esp_netif = esp_netif_next(esp_netif)) != NULL) {
#endif
struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)res_tmp->ai_addr;
memcpy(dns.ip.u_addr.ip6.addr, &ipv6->sin6_addr, sizeof(dns.ip.u_addr.ip6.addr));

Check warning

Code scanning / clang-tidy

Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling] Warning

Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
@euripedesrocha
fix(websocket): Move client to different state when disconnecting
0d8f2a6 
If Websocket client is set to not reconnect, we move to Unknown state to
clean up the task after dispatching disconnected event.
@euripedesrocha euripedesrocha force-pushed the websocket/fix_race_aborting_connection branch from 14cba6e to 0d8f2a6 Compare August 19, 2024 10:53
@david-cermak david-cermak merged commit a353702 into espressif:master Oct 31, 2024
49 checks passed
@johanstokking
Copy link
Contributor

Thanks for this @euripedesrocha.

@johanstokking johanstokking mentioned this pull request Nov 12, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david-cermak david-cermak david-cermak approved these changes

Assignees

@euripedesrocha euripedesrocha

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@euripedesrocha @johanstokking @david-cermak @tomassebestik