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

OS_SocketAccept fails #349

Closed
CDKnightNASA opened this issue Jan 13, 2020 · 5 comments · Fixed by #372 or #401
Closed

OS_SocketAccept fails #349

CDKnightNASA opened this issue Jan 13, 2020 · 5 comments · Fixed by #372 or #401
Assignees
Labels
Milestone

Comments

@CDKnightNASA
Copy link
Contributor

Describe the bug
When using OS_SocketAccept on a stream (TCP) server socket, it fails because the accept code calls OS_ObjectIdGetById() passing a OS_LOCK_MODE_REFCOUNT, but then OS_SocketAccept sees the refcount > 0 and fails out.

To Reproduce
Create code (e.g. an app) that creates a TCP socket server and try using OS_SocketAccept() to receive the incoming connections.

Expected behavior
OS_SocketAccept should work as expected/documented.

Code snips
OS_SockAddr_t Addr;
int Socket = 0, ClientFd;
int32 Status = 0;

OS_SocketAddrInit(&Addr, OS_SocketDomain_INET);
OS_SocketAddrFromString(Addr, "127.0.0.1");
OS_SocketOpen(&Socket, OS_SocketDomain_INET, OS_SocketType_STREAM);
OS_SocketBind(Socket, &Addr);
Status = OS_SocketAccept(Socket, &ClientFd, &Addr, 0);

System observed on:
Debian 9 64-bit x86

Additional context
removing the check for refcount results in expected behavior, OS accept() call should be thread-safe so need not maintain lock through SocketAccept() function.

Reporter Info
Chris Knight, NASA Ames

@skliper
Copy link
Contributor

skliper commented Jan 13, 2020

Is it possible to create a functional test case to cover this? The fact it doesn't work seems to indicate a missing functional test.

@skliper skliper added the bug label Jan 15, 2020
@skliper skliper added this to the 5.1.0 milestone Jan 15, 2020
@CDKnightNASA CDKnightNASA self-assigned this Jan 22, 2020
@skliper
Copy link
Contributor

skliper commented Mar 18, 2020

@CDKnightNASA - any progress on this one? Do you suggest we discuss at a CCB, or is there anything you need to help resolve it?

@CDKnightNASA
Copy link
Contributor Author

I need to write unit tests for it, hasn't been high on priority list. I can take a crack at it today.

@CDKnightNASA
Copy link
Contributor Author

FYI @jphickey there's a great dearth of unit tests for the network API...

So I wasn't clear after this last CCB whether we should use stubbed versions of the OS calls or use the real calls in unit testing. I am guessing that socket code should call the stubs as the OS may throw errors depending on firewall or other security-related configuration.

CDKnightNASA added a commit to CDKnightNASA/osal that referenced this issue Mar 19, 2020
@CDKnightNASA
Copy link
Contributor Author

see #373

@CDKnightNASA CDKnightNASA added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 19, 2020
@astrogeco astrogeco added CCB - 20200325 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Mar 25, 2020
astrogeco added a commit that referenced this issue Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants