<html><body bgcolor="#FFFFFF"><div>Great, that sounds like the right fix. Just update patches 1 and 4 to do that and put it all in a branch and I'll pull it from there. <br><br>Kristian</div><div><br>On Mar 13, 2011, at 4:26 PM, Iskren Chernev <<a href="mailto:iskren.chernev@gmail.com">iskren.chernev@gmail.com</a>> wrote:<br><br></div><div></div><blockquote type="cite"><div>According to the epoll(7) man page, closing a file descriptor removes it from all epoll sets as long as it hasn't been copied (and other copies are not closed):<div><br><div><font class="Apple-style-span" face="'courier new', monospace">Q6  Will  closing  a  file  descriptor cause it to be removed from all</font></div> <div><font class="Apple-style-span" face="'courier new', monospace">    epoll sets automatically?</font></div><div><font class="Apple-style-span" face="'courier new', monospace"><br></font></div><div><font class="Apple-style-span" face="'courier new', monospace">A6  Yes, but . . . (explanation about copied fs via dup* etc)</font></div> <div><br></div><div>So maybe just closing the fds is the cleanest solution, given that only library code can manage them and won't copy them.</div><div><br></div><div>Regards,</div><div>Iskren</div><div><br></div><div class="gmail_quote"> On Sun, Mar 13, 2011 at 9:33 PM, Iskren Chernev <span dir="ltr"><<a href="mailto:iskren.chernev@gmail.com"><a href="mailto:iskren.chernev@gmail.com">iskren.chernev@gmail.com</a></a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> Reply-To-All :)<div><div></div><div class="h5"><br><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Iskren Chernev</b> <span dir="ltr"><<a href="mailto:iskren.chernev@gmail.com" target="_blank"><a href="mailto:iskren.chernev@gmail.com">iskren.chernev@gmail.com</a></a>></span><br> Date: 2011/3/13<br>Subject: Re: minor fixes in event-loop.c<br>To: Kristian Høgsberg <<a href="mailto:krh@bitplanet.net" target="_blank"><a href="mailto:krh@bitplanet.net">krh@bitplanet.net</a></a>><br><br><br>I fixed the "close fd" ones, but its not perfect:<div> -- in what order should we do the epoll_ctl, close and free?</div><div>-- what should we do if epoll_ctl is first and fails, or if close is first and fails (call the next one or return)?</div> <div>-- should we free if epoll_ctl of close fail?</div><div>-- should we loop on close while it returns EINTR - the man page doesn't say weather SA_RESTART works for close.<br><br></div><div>I think that close is unlikely to fail, because it is a special system fd. I'm not sure how often does epoll_ctl fail and what can be done about it -- there is no error returned from epoll_ctl that we might get unless there is something very wrong.</div> <div><br></div><div>It should be good enough for now :)</div><div><br></div><div>Regards,</div><div>Iskren</div><div><br></div><div>PS.: I'll make a github account in the near future. So should I also send the patches with send-mail or only somehow send links to the revision in github?</div> <div><div></div><div> <div><br><div class="gmail_quote">2011/3/13 Kristian Høgsberg <span dir="ltr"><<a href="mailto:krh@bitplanet.net" target="_blank"><a href="mailto:krh@bitplanet.net">krh@bitplanet.net</a></a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div></div><div>On Sun, Mar 13, 2011 at 11:24 AM, Iskren Chernev<br> <<a href="mailto:iskren.chernev@gmail.com" target="_blank"><a href="mailto:iskren.chernev@gmail.com">iskren.chernev@gmail.com</a></a>> wrote:<br> > Some patches for event-loop.c -- the signal & timer fds were never closed.<br> > In the future do you prefer patches standalone or archived?<br> <br> </div></div>Looks like more good patches.  Patches 1 and 4 close the fd before<br> passing it to epoll_ctl(), and I don't think that's how it's supposed<br> to work.  It may or may not work depending on the implementation and<br> the man page doesn't state one way or the other.  However, the obvious<br> safe approach is to just call epoll_ctl() before closing the fd.<br> <br> I prefer patches sent by git send-email, since they're easy to review<br> and discuss on the list, but for more than a couple of patches I also<br> would like a branch somewhere (github, gitorious etc) that I can just<br> pull if it all looks ok.<br> <font color="#888888"><br> Kristian<br> </font></blockquote></div><br></div> </div></div></div><br> </div></div></blockquote></div><br></div> </div></blockquote></body></html>