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

gnrc_sock: warn about non-zero receive timeouts with sock_async #17691

Merged

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 23, 2022

Contribution description

It took me nearly a day of debugging to find out that my event queue was broken due to the handler thread being stuck in a sock_udp_recv(). To prevent that in the future, issue a warning about non-zero-timeouts when the asynchronous callback is set.

Testing procedure

Apply the following patch:

diff --git a/tests/gnrc_sock_async_event/main.c b/tests/gnrc_sock_async_event/main.c
index d77e84c084..93bdc9a485 100644
--- a/tests/gnrc_sock_async_event/main.c
+++ b/tests/gnrc_sock_async_event/main.c
@@ -78,7 +78,7 @@ static void _recv_udp(sock_udp_t *sock, sock_async_flags_t flags, void *arg)
         sock_udp_ep_t remote;
         ssize_t res;
 
-        if ((res = sock_udp_recv(sock, _buffer, sizeof(_buffer), 0,
+        if ((res = sock_udp_recv(sock, _buffer, sizeof(_buffer), 20,
                                  &remote)) >= 0) {
             printf("Received UDP packet from [%s]:%u:\n",
                    ipv6_addr_to_str(_addr_str, (ipv6_addr_t *)remote.addr.ipv6,

Running make -C tests/gnrc_sock_async_event/ -j flash test for any platform should include the warning in the output

gnrc_sock: timeout != 0 with async callback set may lead to unexpected behavior.

It should not show up without the change.

Issues/PRs references

@miri64 miri64 force-pushed the gnrc_sock/enh/warn-about-async-timeout branch from 4c7dc4d to c3bb452 Compare February 23, 2022 16:33
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 24, 2022
@miri64 miri64 enabled auto-merge May 24, 2022 10:09
@miri64 miri64 force-pushed the gnrc_sock/enh/warn-about-async-timeout branch from c3bb452 to 4e88b75 Compare May 24, 2022 10:29
@miri64
Copy link
Member Author

miri64 commented May 24, 2022

Added a comment stemming from offline discussions with @benpicco.

@miri64 miri64 merged commit a2ca69a into RIOT-OS:master May 24, 2022
@miri64 miri64 deleted the gnrc_sock/enh/warn-about-async-timeout branch May 25, 2022 06:33
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants