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

Dropping packets issue with acquiring before push #9

Closed
ip1996 opened this issue Oct 26, 2020 · 5 comments
Closed

Dropping packets issue with acquiring before push #9

ip1996 opened this issue Oct 26, 2020 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@ip1996
Copy link
Collaborator

ip1996 commented Oct 26, 2020

Dropping packets automatically behaves unexpected when the available Packets are Acquired already, but haven't been Pushed yet.

For details check: #8 test/PolledChannelTest.cpp file PolledChannelDropPacketBeforePushTest test case.

Reason:
The freepackets is already empty, however, because the pushing of the packets haven't happened yet, the queuedpackets is also empty, so the Release method will return back with a nullptr when it tries to drop and free packet.

@ip1996 ip1996 added the bug Something isn't working label Oct 26, 2020
@gerazo
Copy link
Owner

gerazo commented Oct 26, 2020

This is a real bug. I have tried to give a fix in #8

@ip1996
Copy link
Collaborator Author

ip1996 commented Oct 26, 2020

Going through the implementation it's rather a consequence of the shared packetpool than a bug. Because at once a resource can only connect to one PacketToFill object, but if our PacketToFill objects haven't been pushed yet, and the channel runs out of free packets, then for the next Acquire it will return back with a null package. So for now, when we use Acquire we should always check that the returned PacketToFill object is empty or not.

@gerazo
Copy link
Owner

gerazo commented Oct 26, 2020

Yes, your are right. However, we should not close this issue until we also put this expected behavior into the documentation on branch docs.

@gerazo
Copy link
Owner

gerazo commented Oct 27, 2020

@BenBlaise You can add the missing documentation (on Channel) to docs branch after you could merge the new master into that branch.

@gerazo
Copy link
Owner

gerazo commented Oct 28, 2020

@BenBlaise I have merged current master into docs. I also put the missing doc lines into the docs. (And I mistakenly put the added extra docs into the merge, but no problem.) So the docs branch is either ready to merge or ready to be continued.

@gerazo gerazo closed this as completed Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants