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.

Sign up for GitHub

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

Jump to bottom

Read/writer filter - If an FD is registered for both read and write filters, deleting it from one will break the other #59

Closed
arr2036 opened this issue Mar 28, 2019 · 1 comment
Closed

Read/writer filter - If an FD is registered for both read and write filters, deleting it from one will break the other #59

arr2036 opened this issue Mar 28, 2019 · 1 comment
Labels
bug

Comments

@arr2036
Copy link
Collaborator

arr2036 commented Mar 28, 2019

I'm not sure if this is accidental or by design, but as both the read and write filters use a single epoll instance, an FD cannot currently be registered for both.

My proposed fix here is to use multiple epoll FDs for the different filters. An alternative approach would be to use a common surrogate knote which is referenced by the read and write filters. The surrogate maintains the state of FD in in epoll, and deletes both read/write knotes if epoll reports an error.
@arr2036 arr2036 added the bug label Mar 28, 2019
@arr2036
Copy link
Collaborator Author

arr2036 commented Mar 28, 2019

This is likely the cause of #29

mheily added a commit that referenced this issue Jan 20, 2020
@mheily
Add a unit test for issue #59
d36b23d
arr2036 pushed a commit that referenced this issue Jun 11, 2020
@mheily @arr2036
Add a unit test for issue #59
7855c56
arr2036 pushed a commit that referenced this issue Jun 11, 2020
@mheily @arr2036
Add a unit test for issue #59
c7e614b
@arr2036 arr2036 changed the title Read/writer filter - If an FD is registered for both read and write filters, deleting it from one can break the other Read/writer filter - If an FD is registered for both read and write filters, deleting it from one will break the other Jun 12, 2020
ghostmansd pushed a commit to ghostmansd/libkqueue that referenced this issue Apr 28, 2021
@mheily @ghostmansd
Add a unit test for issue mheily#59
6f83821
ghostmansd pushed a commit to ghostmansd/libkqueue that referenced this issue Apr 28, 2021
@arr2036 @ghostmansd
Keep a separate state tree for file descriptors inserted into epoll t…
62f7e55 
…o avoid conflicts between EVFILT_READ and EVFILT_WRITE Closes mheily#29 Closes mheily#59

There's more work required, but at least this fixes the outstanding issues.

Known limitations:
- Need to verify correct behaviour of EV_DISPATCH when both filters are registered for events.  This now needs to be implemented internally by libkqueue instead of relying on EPOLLET, as EPOLLET applies to both EPOLLIN and EPOLLOUT.

- If the number of events we are to return does not have sufficient space for at least two entries when polling an FD registered for both read and write events, the write event will be lost.  We can fix this by implementing a linked list of pending events we return before calling the platform's wait function. Will probably copy the dlist implementation from FreeRADIUS across, as it's been well tested, and is a header only implementation like the current rbtree. A dlist is needed to store entries for EVFILT_PROC anyway, which is the next filter that needs major attention.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@arr2036