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

Bug in sockets.c (subsys\net\lib\sockets) #22366

Closed
bran795 opened this issue Jan 31, 2020 · 10 comments · Fixed by #22579
Closed

Bug in sockets.c (subsys\net\lib\sockets) #22366

bran795 opened this issue Jan 31, 2020 · 10 comments · Fixed by #22579
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@bran795
Copy link
Contributor

bran795 commented Jan 31, 2020

Describe the bug
When zsock_accept_ctx is called on a non-blocking socket, after a few times it returns ENFILE - no more file descriptors available

Call
z_free_fd(fd);
Before returning EAGAIN (line 16 of the function accept)

To Reproduce
Steps to reproduce the behavior:

  1. mkdir build; cd build
  2. cmake -DBOARD=frdm_k64f
  3. make
  4. See error

Expected behavior
Accept should return EAGAIN when there is no pending connection on the listening socket

Impact
Showstopper. This bug means non-blocking sockets cannot be used for listening and accepting on the server side.

Screenshots or console output
N/A

Environment (please complete the following information):

  • OS: Windows
  • Toolchain arm mbed
  • Commit SHA or Version used: top as of Jan 30 2020

Additional context
code snippet:

@@ -421,6 +421,7 @@ int zsock_accept_ctx(struct net_context *parent, struct sockaddr *addr,
	ctx = k_fifo_get(&parent->accept_q, timeout);
	if (ctx == NULL) {
		errno = EAGAIN;
+		z_free_fd(fd);
		return -1;
	}
@bran795 bran795 added the bug The issue is a bug, or the PR is fixing a bug label Jan 31, 2020
@jhedberg
Copy link
Member

jhedberg commented Feb 4, 2020

@bran795 are you planning to send a pull request, since you seem to have a fix proposal in the description?

@jhedberg jhedberg added the priority: medium Medium impact/importance bug label Feb 4, 2020
@bran795
Copy link
Contributor Author

bran795 commented Feb 4, 2020

@jhedberg Hi, newbie here
I haven't checked the procedure for sending a pull request
I can send a pull request but it will be a learning process, I'll need some time

@bran795
Copy link
Contributor Author

bran795 commented Feb 6, 2020

@jhedberg
I would appreciate a little help with the pull request
Is there a way to edit the pull request comment after it has been created?
I'm using GitHub Desktop on Windows
Thanks

@pfalcon
Copy link
Contributor

pfalcon commented Feb 6, 2020

@bran795: "Comment", no (well, it won't help). "Commit message", yes. Use git rebase -i master in your branch. Read instructions carefully, and feel free to google it up for more details (Zephyr docs may also include suggestions for development process).

@bran795
Copy link
Contributor Author

bran795 commented Feb 6, 2020

@pfalcon right
what about the Identity/Emails issues? I added an email address first.last@company to my github account but that doesn't seem to work either
thanks

@jhedberg
Copy link
Member

jhedberg commented Feb 6, 2020

@bran795 you need to configure your local git correctly, which is normally done through $HOME/.gitconfig. E.g. in mine I have:

[user]
	name = Johan Hedberg
	email = johan.hedberg@intel.com

Since you already created the commit you'll need to amend its author explicitly. You should be able to do that with git commit --amend --author="Your Name <your@email> or with git commit --amend --reset-author after you've fixed your git configuration.

@pfalcon
Copy link
Contributor

pfalcon commented Feb 6, 2020

@bran795: Name/email in commit's "Author:" field should match those in "Signed-off-by:" as present in the commit message. In #22366 I see (git log):

Author: bran795 <48238460+bran795@users.noreply.github.com>

which suggests that you submitted that PR from Github's web UI. That won't work here, you would need to use command-line git client, I'm afraid.

@bran795
Copy link
Contributor Author

bran795 commented Feb 6, 2020

@jhedberg
@pfalcon
Thanks, I'll install git and try again

@jhedberg
Copy link
Member

jhedberg commented Feb 6, 2020

you submitted that PR from Github's web UI

I had no idea that GitHub had such a feature. I wonder if it can be disabled per-project, since it doesn't seem to be able to produce what's required for Zephyr.

@bran795
Copy link
Contributor Author

bran795 commented Feb 6, 2020

Getting closer...

Gitlint issues
1: UC2 Signed-off-by: must have a full name

Identity/Emails issues
1a868e8: author email (Inbar Anson Bratspiess inbar.anson.bratspiess@330plus.net) needs to match one of the signed-off-by entries.

Please help

Thanks

jukkar pushed a commit to jukkar/zephyr that referenced this issue Feb 7, 2020
The zsock_accept_ctx() calls z_reserve_fd() on entry but fails
to call z_free_fd() on failure. This will leak the allocated
socket descriptor.

Fixes zephyrproject-rtos#22366

Signed-off-by: Inbar Anson Bratspiess <inbar.anson.bratspiess@330plus.net>
jukkar pushed a commit that referenced this issue Feb 10, 2020
The zsock_accept_ctx() calls z_reserve_fd() on entry but fails
to call z_free_fd() on failure. This will leak the allocated
socket descriptor.

Fixes #22366

Signed-off-by: Inbar Anson Bratspiess <inbar.anson.bratspiess@330plus.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
6 participants