9 ms·

Epoll is broken – part 2/2

108 points by protomyth 10y ago

jgh 10y ago
Interesting writeup, seems like this only applies to people writing epoll abstraction layers though. Everybody else is able to avoid this bug by deregistering the fds before calling close. So I'm not sure calling it "fundamentally broken" is necessarily accurate.
- bluejekyll 10y ago
  Error prone? It definitely creates situations where bugs crop up. The example of close() is actually quite important, and something which is perhaps unexpected until you hit that bug and spend hours or days tracking it down. Definitely has happened to me.
- joosters 10y ago
  Yeah, I think 'fundamentally broken' is going into clickbait territory - these are API calls that are successfully used by a huge number of programs. epoll() is the most recent of these multiplexer syscalls and it's been around for a decade or so. The underlying message is that there are corner cases that exist and should be thought about, but that is true of most syscalls. The epoll() call itself is a strange API. As the series of articles says, it was introduced to solve the problem of the kernel having to register and unregister every listed FD on every invocation. However, as a trade-off, your program now needs to make a syscall each time it wants to add/remove an FD. So where you once had a single poll() call per event loop, you now have to fire off extra syscalls for each single FD on each new connection. Compare this to the API of FreeBSD's kqueue - it is another solution to avoid the linear costs of select()/poll(), but instead of making a syscall for registering/unregistering each new FD, kevent() is called with a list of FDs that you want to change. So an event loop using kqueue consists of just one syscall per loop.
  - zrm 10y ago
    > Compare this to the API of FreeBSD's kqueue - it is another solution to avoid the linear costs of select()/poll(), but instead of making a syscall for registering/unregistering each new FD, kevent() is called with a list of FDs that you want to change. So an event loop using kqueue consists of just one syscall per loop. It would be simple enough to add a version of epoll_ctl that took array arguments, but the existing epoll_ctl makes a lot of sense for the most common use cases. You either know all the descriptors ahead of time or you don't. If you do then you can add them all on startup and never touch it again, and one-time costs usually aren't worth optimizing. If you don't then you're usually modifying them one at a time anyway. And if the syscall takes them one at a time then you don't have to worry about maintaining the state yourself or memory management based on how many descriptors you have or anything like that.
- asveikau 10y ago
  It sounds like you can avoid the bug except you know, the part where you can't avoid bugs. Example: Let's say your program is multi-threaded and you have a bug where you close(2) a dangling file descriptor, which normally harmlessly returns EBADF but this time its integer value happens due to unlucky timing to get re-used for a brand-new socket which is registered with epoll... Then suddenly perhaps this becomes relevant in your program, even though any time you intentionally close the fd you de-register from epoll first. In other words, when debugging a program using epoll, I think it's still a relevant question. These corner-case type things where it seems like "obviously" you're never going to screw up are often where the most interesting bugs can surface.
  - quotemstr 10y ago
    If you ever see EBADF, abort the program. You've screwed up something badly somewhere. Ignoring EBADF is like ignoring a double free: even if you can get away with it in the short term, it's indicative of something horrible down the road.
    - asveikau 10y ago
      Yes, for the reasons I already illustrated in my example. To be clear I am not advocating EBADF to be something you should live with. I am describing it as unintentional and as a bug. It is a time bomb for harmful race conditions.
- naasking 10y ago
  > Interesting writeup, seems like this only applies to people writing epoll abstraction layers though. [...] So I'm not sure calling it "fundamentally broken" is necessarily accurate. If you can't write an abstraction around it, it sounds pretty broken.
- mfukar 10y ago
  The bug is only there because the epoll interface necessitates you write the code to avoid it. That, to me, is the definition of a fundamentally broken interface.
jack9 10y ago
> had shedded some light Grammar.
mnarayan01 10y ago
I would think you could use dup (or dup2/ioctl) to de-register (assuming nothing grabbed that FD)?
- DSMan195276 10y ago
  That could possibly work, but only if you dup() the same original FD. If you just try dup()ing any random FD the internal file description pointer won't match, so epoll won't reconize it. But of course, if you have any threads then it's a big race condition.
al452 10y ago
"Using these toolkits is like trying to make a bookshelf out of mashed potatoes." - Jamie Zawinski
mbrumlow 10y ago
From the man page... > Q6 Will closing a file descriptor cause it to be removed from all epoll sets automatically? > A6 Yes, but be aware of the following point. A file descriptor is a reference to an open file description (see open(2)). Whenever a file descriptor is duplicated via dup(2), dup2(2), > fcntl(2) F_DUPFD, or fork(2), a new file descriptor referring to the same open file description is created. An open file description continues to exist until all file descriptors refer‐ > ring to it have been closed. A file descriptor is removed from an epoll set only after all the file descriptors referring to the underlying open file description have been closed (or > before if the file descriptor is explicitly removed using epoll_ctl(2) EPOLL_CTL_DEL). This means that even after a file descriptor that is part of an epoll set has been closed, events > may be reported for that file descriptor if other file descriptors referring to the same underlying file description remain open. EDIT: Lots of comments here seem to think this should be unexpected or is a bug. Closing a FD you are using is a bug. I think epoll does a fairly good job of letting the user know that it is watching the description and not the descriptor. Failing to read the man page for dup would also leave you in a blind spot. I have been writing code for linux a while now and I did not think it was any secret that a file is still open until all of the fds pointing to it are closed. That is why you have to take care and close your duplicated fds at the right time otherwise you will end up with file handles leaking. The example code provided illustrates this perfectly. As a side note using dup2 to get your original FD passed to epoll associated with the still open description from the duped fd should allow you to remove it.
- ajross 10y ago
  > Closing a FD you are using is a bug. Sort of, but think of the problem epoll is addressing: having an epoll-registered descriptor is semantically equivalent to having an active read() on the thing. That's sort of the whole point, of course. Calling close() on a descriptor blocked in read() works fine and produces the expected (synchronous EOF) behavior. Doing it on the epoll one produces surprising behavior depending on whether or not it got dup'd somewhere (which can have been somewhere outside your process!). It's a bug for sure. Not sure it rises to the level of "fundamentally broken", but it's a bug.
int_19h 10y ago
I always found it interesting that WinNT got this one right very early on (IOCP; first appeared in NT 3.5 in 1994), and thus avoided all these problems, and a series of subsequent APIs trying to correct them. The way you do efficient asynchronous I/O in Win32 today is still the same as it was 20 years ago. For those unfamiliar with that model, here's a brief description: https://web.archive.org/web/20101101112358/http://doc.sch130.nsc.ru/www.sysinternals.com/ntw2k/info/comport.shtml https://web.archive.org/web/20101101112358/http://doc.sch130... My understanding is that kqueue in FreeBSD is also conceptually similar, but I never had a chance to take a close look at that.
- pacaro 10y ago
  The NT team had the advantage that they (Cutler et al) had worked on a previous OS and had the opportunity to revisit design decisions in many places. I have no idea if/how VMS solved this problem
  - masklinn 10y ago
    While that can be (and is, really) an excuse for select(2) and poll(2) (which I believe date back to '83) it is not one for epoll (October 2002) as it was preceded by both IOCP ('94) and kqueue (July 2000).
- jstarks 10y ago
  I dunno, NT tied IOs to the initiating thread until Vista. This means that you had to keep the initiating thread around or your IOs would be cancelled. NT also has the same flaw mentioned in the article; it ties the IOCP registration to the file object instead of the file handle. This means that you can't use a handle with an IOCP in one process then hand it off to another process. The NT IO model has some nice properties but also some pretty serious warts. I think no one got it right.
  - wahern 10y ago
    kqueue got it right. Not just wrt descriptors but with many other aspects of the API. The BSDs tend to put more thought and abstraction into their designs. Linux and Windows designs tend to be primarily driven by the [expected] most common use case, which often leads to premature design and performance optimizations that over the years prove ill conceived or shortsighted as programming patterns shift. OTOH, Linux and Windows APIs tend to be more immediately useable. On BSD things tend to be more "some assembly required". That's all very general but I have very specific examples in mind, like IOCP vs polling, signalfd vs EVFILT_SIGNAL, inotify v EVFILT_VNODE, containers v jail, and seccomp v pledge[1], others. [1] With seccomp v pledge, seccomp seems like it needs more assembly. But I think the most common expected use case, given that seccomp restrictions are inherited across fork, was that seccomp sandboxes would be created by a core system utility which then invoked other services, sandboxing them without having to modify the services. pledge requires source code modification. pledge, I think, is the better and more useful approach but it necessarily requires some assembly. Same story for containers vs jail, at least until recently.
deleted 10y ago
[deleted]
mbrumlow 10y ago
I am making a new post because I feel like this is getting out of hand. There is a lot of misinformation that needs to be cleared up. When you are working with system calls you use fds to tell the kernel what file you are working with. These are simply numbers, an int to be exact. When you call a system call that takes in a fd you pass the int that references the file you want to work with and it is looked up in a table to find the kernel structure that represents the file. When you dup a fd all that does is add a entry to the lookup table to point to the same kernel structure -- increasing the refcount at the same time. The ref count lives on the kernel structure, not with the int. The fd system is real simple. For open, dupe, dup2… will add an entry. While close will remove the entry. If the entry is removed the fd is now useless for any communication to the kernel over system calls. This is because the lookup table will not be able to translate your fd to a kernel structure. Now the question is epoll broken or have a bug? I don’t think so. You just have to keep in mind you are using a system call and not a library. This is controlled access to kernel level functions. In the example adding a fd to epoll, then duping it followed by a closing is providing some surprising results. Understanding how userspace talks to kernel space and how system calls translate into running kernel code is key to understanding why this is not a bug but just how things work. You might also be surprised that other system calls behave the same way! When you tell epoll to wait you use a fd to tell it which file you want to wait on. You are not telling a library, or a userspace program to do this. You are telling the kernel. So the fd is translated into a kernel object and that is what epoll is working with. Only when adding and removing is this translation done. The kernel subsystem that provides epoll will continue to work with kernel objects -- not fds. When you call dup you are adding a new entry to your programs fd lookup table for a new fd but pointing to the same kernel structure -- at the same time you are also increment the refcount of the kernel structure. The file is not actually closed until both the original fd and the duped fd have had closed called on it, or more correctly, until the refcount of the kernel structure is zero. This means that if you tell epoll to wait on a file and do so by an fd, and then close the fd -- you will only see a close event if the refcount is zero. By calling dup before closing your original fd you have incremented the ref count and thus the file remains open -- no close event. If you then try to remove your file from epoll using the now invalid (closed) fd it will fail. This is because the fd is no longer in the fd lookup table and the system call can not translate your fd into a kernel object. Now you might say this is not how other system calls work. As in another thread pointed out that “read” does not behave this way. To the contrary -- read actually works the same way. If you dup your socket fd, and use a thread to close your socket fd while in an active read -- your read will not fail. It will finish out delivering the data to you. This is because the fd is not the actual object you are working with. It is just a reference to the object. Onces you enter the read function you are in kernel land. And the kernel only knows about the kernel object. Because you duped the fd before closing it the kernel object has a ref count of 2 and your close while read leaves the ref count at 1 thus there is nothing for the kernel to do other than finish out its read. If you attempt to use the now closed and invalid fd for a subsequent read, or close you will get a EBADF error -- much like you do when you attempt to use it with epolls functions to remove the file.
ericbb 10y ago
For people reading this thread who want to better understand the distinctions between files, file descriptions, and file descriptors, I recommend Michael Kerrisk's book The Linux Programming Interface, which has great coverage of this topic.
protomyth 10y ago
https://illumos.org/man/5/epoll https://illumos.org/man/5/epoll While a best effort has been made to mimic the Linux semantics, there are some semantics that are too peculiar or ill-conceived to merit accommodation. In particular, the Linux epoll facility will -- by design -- continue to generate events for closed file descriptors where/when the underlying file description remains open. For example, if one were to fork(2) and subsequently close an actively epoll'd file descriptor in the parent, any events generated in the child on the implicitly duplicated file descriptor will continue to be delivered to the parent -- despite the fact that the parent itself no longer has any notion of the file description! This epoll facility refuses to honor these semantics; closing the EPOLL_CTL_ADD'd file descriptor will always result in no further events being generated for that event description.

108 points by protomyth 10y ago

jgh 10y ago

Interesting writeup, seems like this only applies to people writing epoll abstraction layers though. Everybody else is able to avoid this bug by deregistering the fds before calling close. So I'm not sure calling it "fundamentally broken" is necessarily accurate.

bluejekyll 10y ago
Error prone? It definitely creates situations where bugs crop up. The example of close() is actually quite important, and something which is perhaps unexpected until you hit that bug and spend hours or days tracking it down. Definitely has happened to me.
joosters 10y ago
Yeah, I think 'fundamentally broken' is going into clickbait territory - these are API calls that are successfully used by a huge number of programs. epoll() is the most recent of these multiplexer syscalls and it's been around for a decade or so. The underlying message is that there are corner cases that exist and should be thought about, but that is true of most syscalls. The epoll() call itself is a strange API. As the series of articles says, it was introduced to solve the problem of the kernel having to register and unregister every listed FD on every invocation. However, as a trade-off, your program now needs to make a syscall each time it wants to add/remove an FD. So where you once had a single poll() call per event loop, you now have to fire off extra syscalls for each single FD on each new connection. Compare this to the API of FreeBSD's kqueue - it is another solution to avoid the linear costs of select()/poll(), but instead of making a syscall for registering/unregistering each new FD, kevent() is called with a list of FDs that you want to change. So an event loop using kqueue consists of just one syscall per loop.
- zrm 10y ago
  > Compare this to the API of FreeBSD's kqueue - it is another solution to avoid the linear costs of select()/poll(), but instead of making a syscall for registering/unregistering each new FD, kevent() is called with a list of FDs that you want to change. So an event loop using kqueue consists of just one syscall per loop. It would be simple enough to add a version of epoll_ctl that took array arguments, but the existing epoll_ctl makes a lot of sense for the most common use cases. You either know all the descriptors ahead of time or you don't. If you do then you can add them all on startup and never touch it again, and one-time costs usually aren't worth optimizing. If you don't then you're usually modifying them one at a time anyway. And if the syscall takes them one at a time then you don't have to worry about maintaining the state yourself or memory management based on how many descriptors you have or anything like that.
asveikau 10y ago
It sounds like you can avoid the bug except you know, the part where you can't avoid bugs. Example: Let's say your program is multi-threaded and you have a bug where you close(2) a dangling file descriptor, which normally harmlessly returns EBADF but this time its integer value happens due to unlucky timing to get re-used for a brand-new socket which is registered with epoll... Then suddenly perhaps this becomes relevant in your program, even though any time you intentionally close the fd you de-register from epoll first. In other words, when debugging a program using epoll, I think it's still a relevant question. These corner-case type things where it seems like "obviously" you're never going to screw up are often where the most interesting bugs can surface.
- quotemstr 10y ago
  If you ever see EBADF, abort the program. You've screwed up something badly somewhere. Ignoring EBADF is like ignoring a double free: even if you can get away with it in the short term, it's indicative of something horrible down the road.
  - asveikau 10y ago
    Yes, for the reasons I already illustrated in my example. To be clear I am not advocating EBADF to be something you should live with. I am describing it as unintentional and as a bug. It is a time bomb for harmful race conditions.
naasking 10y ago
> Interesting writeup, seems like this only applies to people writing epoll abstraction layers though. [...] So I'm not sure calling it "fundamentally broken" is necessarily accurate. If you can't write an abstraction around it, it sounds pretty broken.
mfukar 10y ago
The bug is only there because the epoll interface necessitates you write the code to avoid it. That, to me, is the definition of a fundamentally broken interface.

jack9 10y ago

> had shedded some light Grammar.

mnarayan01 10y ago

I would think you could use dup (or dup2/ioctl) to de-register (assuming nothing grabbed that FD)?

DSMan195276 10y ago
That could possibly work, but only if you dup() the same original FD. If you just try dup()ing any random FD the internal file description pointer won't match, so epoll won't reconize it. But of course, if you have any threads then it's a big race condition.

al452 10y ago

"Using these toolkits is like trying to make a bookshelf out of mashed potatoes." - Jamie Zawinski

mbrumlow 10y ago

From the man page... > Q6 Will closing a file descriptor cause it to be removed from all epoll sets automatically? > A6 Yes, but be aware of the following point. A file descriptor is a reference to an open file description (see open(2)). Whenever a file descriptor is duplicated via dup(2), dup2(2), > fcntl(2) F_DUPFD, or fork(2), a new file descriptor referring to the same open file description is created. An open file description continues to exist until all file descriptors refer‐ > ring to it have been closed. A file descriptor is removed from an epoll set only after all the file descriptors referring to the underlying open file description have been closed (or > before if the file descriptor is explicitly removed using epoll_ctl(2) EPOLL_CTL_DEL). This means that even after a file descriptor that is part of an epoll set has been closed, events > may be reported for that file descriptor if other file descriptors referring to the same underlying file description remain open. EDIT: Lots of comments here seem to think this should be unexpected or is a bug. Closing a FD you are using is a bug. I think epoll does a fairly good job of letting the user know that it is watching the description and not the descriptor. Failing to read the man page for dup would also leave you in a blind spot. I have been writing code for linux a while now and I did not think it was any secret that a file is still open until all of the fds pointing to it are closed. That is why you have to take care and close your duplicated fds at the right time otherwise you will end up with file handles leaking. The example code provided illustrates this perfectly. As a side note using dup2 to get your original FD passed to epoll associated with the still open description from the duped fd should allow you to remove it.

ajross 10y ago
> Closing a FD you are using is a bug. Sort of, but think of the problem epoll is addressing: having an epoll-registered descriptor is semantically equivalent to having an active read() on the thing. That's sort of the whole point, of course. Calling close() on a descriptor blocked in read() works fine and produces the expected (synchronous EOF) behavior. Doing it on the epoll one produces surprising behavior depending on whether or not it got dup'd somewhere (which can have been somewhere outside your process!). It's a bug for sure. Not sure it rises to the level of "fundamentally broken", but it's a bug.

int_19h 10y ago

I always found it interesting that WinNT got this one right very early on (IOCP; first appeared in NT 3.5 in 1994), and thus avoided all these problems, and a series of subsequent APIs trying to correct them. The way you do efficient asynchronous I/O in Win32 today is still the same as it was 20 years ago. For those unfamiliar with that model, here's a brief description: https://web.archive.org/web/20101101112358/http://doc.sch130.nsc.ru/www.sysinternals.com/ntw2k/info/comport.shtml https://web.archive.org/web/20101101112358/http://doc.sch130... My understanding is that kqueue in FreeBSD is also conceptually similar, but I never had a chance to take a close look at that.

pacaro 10y ago
The NT team had the advantage that they (Cutler et al) had worked on a previous OS and had the opportunity to revisit design decisions in many places. I have no idea if/how VMS solved this problem
- masklinn 10y ago
  While that can be (and is, really) an excuse for select(2) and poll(2) (which I believe date back to '83) it is not one for epoll (October 2002) as it was preceded by both IOCP ('94) and kqueue (July 2000).
jstarks 10y ago
I dunno, NT tied IOs to the initiating thread until Vista. This means that you had to keep the initiating thread around or your IOs would be cancelled. NT also has the same flaw mentioned in the article; it ties the IOCP registration to the file object instead of the file handle. This means that you can't use a handle with an IOCP in one process then hand it off to another process. The NT IO model has some nice properties but also some pretty serious warts. I think no one got it right.
- wahern 10y ago
  kqueue got it right. Not just wrt descriptors but with many other aspects of the API. The BSDs tend to put more thought and abstraction into their designs. Linux and Windows designs tend to be primarily driven by the [expected] most common use case, which often leads to premature design and performance optimizations that over the years prove ill conceived or shortsighted as programming patterns shift. OTOH, Linux and Windows APIs tend to be more immediately useable. On BSD things tend to be more "some assembly required". That's all very general but I have very specific examples in mind, like IOCP vs polling, signalfd vs EVFILT_SIGNAL, inotify v EVFILT_VNODE, containers v jail, and seccomp v pledge[1], others. [1] With seccomp v pledge, seccomp seems like it needs more assembly. But I think the most common expected use case, given that seccomp restrictions are inherited across fork, was that seccomp sandboxes would be created by a core system utility which then invoked other services, sandboxing them without having to modify the services. pledge requires source code modification. pledge, I think, is the better and more useful approach but it necessarily requires some assembly. Same story for containers vs jail, at least until recently.

deleted 10y ago

[deleted]

mbrumlow 10y ago

I am making a new post because I feel like this is getting out of hand. There is a lot of misinformation that needs to be cleared up. When you are working with system calls you use fds to tell the kernel what file you are working with. These are simply numbers, an int to be exact. When you call a system call that takes in a fd you pass the int that references the file you want to work with and it is looked up in a table to find the kernel structure that represents the file. When you dup a fd all that does is add a entry to the lookup table to point to the same kernel structure -- increasing the refcount at the same time. The ref count lives on the kernel structure, not with the int. The fd system is real simple. For open, dupe, dup2… will add an entry. While close will remove the entry. If the entry is removed the fd is now useless for any communication to the kernel over system calls. This is because the lookup table will not be able to translate your fd to a kernel structure. Now the question is epoll broken or have a bug? I don’t think so. You just have to keep in mind you are using a system call and not a library. This is controlled access to kernel level functions. In the example adding a fd to epoll, then duping it followed by a closing is providing some surprising results. Understanding how userspace talks to kernel space and how system calls translate into running kernel code is key to understanding why this is not a bug but just how things work. You might also be surprised that other system calls behave the same way! When you tell epoll to wait you use a fd to tell it which file you want to wait on. You are not telling a library, or a userspace program to do this. You are telling the kernel. So the fd is translated into a kernel object and that is what epoll is working with. Only when adding and removing is this translation done. The kernel subsystem that provides epoll will continue to work with kernel objects -- not fds. When you call dup you are adding a new entry to your programs fd lookup table for a new fd but pointing to the same kernel structure -- at the same time you are also increment the refcount of the kernel structure. The file is not actually closed until both the original fd and the duped fd have had closed called on it, or more correctly, until the refcount of the kernel structure is zero. This means that if you tell epoll to wait on a file and do so by an fd, and then close the fd -- you will only see a close event if the refcount is zero. By calling dup before closing your original fd you have incremented the ref count and thus the file remains open -- no close event. If you then try to remove your file from epoll using the now invalid (closed) fd it will fail. This is because the fd is no longer in the fd lookup table and the system call can not translate your fd into a kernel object. Now you might say this is not how other system calls work. As in another thread pointed out that “read” does not behave this way. To the contrary -- read actually works the same way. If you dup your socket fd, and use a thread to close your socket fd while in an active read -- your read will not fail. It will finish out delivering the data to you. This is because the fd is not the actual object you are working with. It is just a reference to the object. Onces you enter the read function you are in kernel land. And the kernel only knows about the kernel object. Because you duped the fd before closing it the kernel object has a ref count of 2 and your close while read leaves the ref count at 1 thus there is nothing for the kernel to do other than finish out its read. If you attempt to use the now closed and invalid fd for a subsequent read, or close you will get a EBADF error -- much like you do when you attempt to use it with epolls functions to remove the file.

ericbb 10y ago

For people reading this thread who want to better understand the distinctions between files, file descriptions, and file descriptors, I recommend Michael Kerrisk's book The Linux Programming Interface, which has great coverage of this topic.

protomyth 10y ago

https://illumos.org/man/5/epoll https://illumos.org/man/5/epoll While a best effort has been made to mimic the Linux semantics, there are some semantics that are too peculiar or ill-conceived to merit accommodation. In particular, the Linux epoll facility will -- by design -- continue to generate events for closed file descriptors where/when the underlying file description remains open. For example, if one were to fork(2) and subsequently close an actively epoll'd file descriptor in the parent, any events generated in the child on the implicitly duplicated file descriptor will continue to be delivered to the parent -- despite the fact that the parent itself no longer has any notion of the file description! This epoll facility refuses to honor these semantics; closing the EPOLL_CTL_ADD'd file descriptor will always result in no further events being generated for that event description.