public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: Tony Battersby <tonyb@cybernetics.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Davide Libenzi <davidel@xmailserver.org>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] [2.6.29] epoll: fix for epoll_wait sometimes returning events on closed fds
Date: Tue, 24 Feb 2009 20:52:11 +0100	[thread overview]
Message-ID: <49A44FEB.6000803@cosmosbay.com> (raw)
In-Reply-To: <49A43FF0.2070703@cybernetics.com>

Tony Battersby a écrit :
> Eric Dumazet wrote:
>> Your patch may solve part of the problem. In your programs, maybe you
>> have one thread doing all epoll_wait() and close() syscalls, but what
>> of other programs ?
>>
>> What prevents a thread doing close(fd) right after an other thread
>> got this fd from epoll_wait() ?
>> Nothing, and application may strangely react.
>>
>> The moment you have several threads doing read()/write()/close() syscalls
>> on the same fd at the same time, you obviously get problems, not
>> only with epoll.
>>
>> In a typical epoll driven application, with a pool of N worker threads all doing :
>>
>> while (1) {
>> 	fd = epoll_wait(epoll_fd);
>> 	work_on_fd(fd); /* possibly calling close(fd); */
>> }
>>
>> Then, you must be prepared to get a *false* event, ie an fd that another worker
>> already closed (and eventually reopened)
>>
>>
>>
>>   
> Yes, I agree that userspace threads do need synchronization to prevent
> one thread from stomping on another thread's data.  If userspace can't
> prove that close() returned before the call to epoll_wait(), then
> epoll_wait() may legitimately return an event on a closed fd.  That's
> why my test program did close() and then epoll_wait() from the same
> thread - to prove that they were serialized.
> 
> I am not actually using epoll in any of my programs right now; I was
> just investigating a bug reported to me by another programmer at my
> company.  So my test program isn't intented to reflect anything other
> than a way to reproduce the problem reliably.  However, I can imagine
> that a network program might want to spawn separate rx/tx threads on the
> same socket, in which case it might make sense to have separate threads
> accessing the same file descriptor.  As you say, the two threads would
> have to use proper locking, but that is purely a userspace issue that
> kernel developers need not worry about.  So I am only concerned with the
> case that userspace can prove that close() and epoll_wait() were
> properly serialized, and epoll_wait() still returned an event on the
> closed fd.

I believe Davide already said that close() meant the real close, not
the file descriptor freeing. 

And current documentations clearly states it, maybe your man page is too old ?

---

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 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 referring 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 descriptor is
    explicitly removed using epoll_ctl() 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.




  reply	other threads:[~2009-02-24 19:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-24 17:26 [PATCH 1/6] [2.6.29] epoll: fix for epoll_wait sometimes returning events on closed fds Tony Battersby
2009-02-24 18:12 ` Eric Dumazet
2009-02-24 18:44   ` Tony Battersby
2009-02-24 19:52     ` Eric Dumazet [this message]
2009-02-24 20:36       ` Tony Battersby
2009-02-24 20:45         ` Davide Libenzi
2009-02-24 20:52           ` Tony Battersby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49A44FEB.6000803@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=davidel@xmailserver.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tonyb@cybernetics.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox