-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
net: sockets: services: Don't modify pollfd array from other threads #75525
Merged
aescolar
merged 1 commit into
zephyrproject-rtos:main
from
rlubos:net/socket-services-odd-test-failure
Jul 9, 2024
Merged
net: sockets: services: Don't modify pollfd array from other threads #75525
aescolar
merged 1 commit into
zephyrproject-rtos:main
from
rlubos:net/socket-services-odd-test-failure
Jul 9, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pollfd array used with zsock_poll() should not be modified while inside zsock_poll() function as this could lead to unexpected results. For instance, k_poll already monitoring some kernel primitive could report an event, but it will not be processed if the monitored socket file descriptor in the pollfd array was set to -1. In result, zsock_poll() may unexpectedly quit prematurely, returning 0 events, even if it was requested to wait infinitely. The pollfd arrays used by zsock_poll() (ctx.events) is reinitialized when the service thread is restarted so modifying it directly when registering/unregistering service is not really needed. It's enough if those functions notify the eventfd socket used to restart the services thread. Signed-off-by: Robert Lubos <[email protected]>
jukkar
approved these changes
Jul 5, 2024
pdgendt
approved these changes
Jul 5, 2024
jukkar
added a commit
to jukkar/zephyr
that referenced
this pull request
Jul 7, 2024
As found in PR zephyrproject-rtos#75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
katgiadla
approved these changes
Jul 8, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works on Nordic targets
aescolar
pushed a commit
that referenced
this pull request
Jul 10, 2024
As found in PR #75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
AlienSarlak
pushed a commit
to AlienSarlak/zephyr
that referenced
this pull request
Jul 13, 2024
As found in PR zephyrproject-rtos#75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
CZKikin
pushed a commit
to nxp-upstream/zephyr
that referenced
this pull request
Jul 18, 2024
As found in PR zephyrproject-rtos#75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
Devansh0210
pushed a commit
to Devansh0210/zephyr
that referenced
this pull request
Jul 20, 2024
As found in PR zephyrproject-rtos#75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
mpenate-ellenbytech
pushed a commit
to mpenate-ellenbytech/zephyr
that referenced
this pull request
Aug 19, 2024
As found in PR zephyrproject-rtos#75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. Signed-off-by: Jukka Rissanen <[email protected]>
Chenhongren
pushed a commit
to Chenhongren/zephyr
that referenced
this pull request
Aug 26, 2024
As found in PR zephyrproject-rtos#75525, we should not modify the polled fd array in multiple places. Because of this fix, the async version of the socket service could start to trigger while it is being handled by the async handler. This basically means that the async version cannot work as intended so remove its support. (cherry picked from commit 794d7cf) Original-Signed-off-by: Jukka Rissanen <[email protected]> GitOrigin-RevId: 794d7cf Change-Id: I049890a7827d3a03f245abf171877f4ab0304e15 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5691930 Tested-by: Keith Short <[email protected]> Reviewed-by: Keith Short <[email protected]> Commit-Queue: Keith Short <[email protected]> Tested-by: ChromeOS Prod (Robot) <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pollfd array used with zsock_poll() should not be modified while inside zsock_poll() function as this could lead to unexpected results. For instance, k_poll already monitoring some kernel primitive could report an event, but it will not be processed if the monitored socket file descriptor in the pollfd array was set to -1. In result, zsock_poll() may unexpectedly quit prematurely, returning 0 events, even if it was requested to wait infinitely.
The pollfd arrays used by zsock_poll() (ctx.events) is reinitialized when the service thread is restarted so modifying it directly when registering/unregistering service is not really needed. It's enough if those functions notify the eventfd socket used to restart the services thread.
Fixes #75474