<div dir="ltr">I seem that my patch is duplicate of <a href="https://reviews.llvm.org/D69575">https://reviews.llvm.org/D69575</a>, It's just I covered some corner cases.<div>I think the best way to solve this is to cover some race conditions in D69575 or simply ignore those corner cases if those are unlikely to happen.</div><div>I left some comments with explanations in <a href="https://reviews.llvm.org/D69575">https://reviews.llvm.org/D69575</a></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Nov 22, 2019 at 11:06 PM TT KILEW <<a href="mailto:tt.kilew@gmail.com">tt.kilew@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Well, Yes. <div>There are a few differences with the <a href="https://reviews.llvm.org/D70563" target="_blank">https://reviews.llvm.org/D70563</a> solution.</div><div>First of all, in D70563, in case if file will be deleted between `open` and subscription to events, then there'll be maximum timeout. it seems that kqueue is not validating file descriptor, so, in case if file descriptor is already invalid, listening for a kqueue won't give any errors. It will just wait for a timeout and won't receive any events.</div><div>This behavior can be simply simulated in the code sample, provided in <a href="https://reviews.llvm.org/D70563" target="_blank">https://reviews.llvm.org/D70563</a></div><div>  </div><div>This is why in <a href="https://reviews.llvm.org/D70563" target="_blank">D70563</a> registration to the queue and listening to are two different calls. So logic in the path is</div><div>1) Register for events  (now queue will  receive events, but we haven't setup listeners to it yet)</div><div>2) Check if lock file was deleted</div><div>3) Start receiving events</div><div><br></div><div>This logic allows preventing race condition if the file was deleted in between open/subscription calls.</div><div>It's very unlikely to have this race condition, but 90 seconds is pretty long time.</div><div><br></div><div>Second. in  <a href="https://reviews.llvm.org/D70563" target="_blank">D70563</a> there's an additional event we're listening to check if the process, lock owner has exited. So, in case if owner has died, but for some reason hasn't deleted lock file, we'll exit from the waiting queue.</div><div>D69575 solution will still wait for file to be deleted for a whole 90seoonds timeout.<br></div><div>I'm not sure, how often this lock owner dies without unlocking, but this was in the original code, so I implemented this part as well.<br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Nov 22, 2019 at 10:20 PM Michael Spencer <<a href="mailto:bigcheesegs@gmail.com" target="_blank">bigcheesegs@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div><div dir="ltr">On Fri, Nov 22, 2019 at 10:59 AM TT KILEW via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi there. I submitted a patch I llvm  that fixed polling logic in LockFileManager<div><a href="https://reviews.llvm.org/D70563" target="_blank">https://reviews.llvm.org/D70563</a></div><div><br></div><div>This patch should fix </div><div><a href="https://bugs.llvm.org/show_bug.cgi?id=20794" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=20794</a></div><div><br></div><div>There were no activity so I decided to write directly  to mail list<br><div><br></div><div>Is there anyone who can take a look? Thanks.</div><div><br></div><div>P.S. Are there some additional actions needed for patch to be reviewed? Thanks</div></div></div></blockquote><div><br></div><div>I recently reviewed <a href="https://reviews.llvm.org/D69575" target="_blank">https://reviews.llvm.org/D69575</a> which solves the same problem and seems simpler. Is there anything in your patch not covered by D69575?</div><div><br></div><div>- Michael Spencer</div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Best Regards,<br>Павло Тайкало  / Paul Taykalo<br>skype: tt.kilew, tel.: +38 097 86 1024 1<br><a href="http://twitter.com/tt_kilew" target="_blank">http://twitter.com/tt_kilew</a><br></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Best Regards,<br>Павло Тайкало  / Paul Taykalo<br>skype: tt.kilew, tel.: +38 097 86 1024 1<br><a href="http://twitter.com/tt_kilew" target="_blank">http://twitter.com/tt_kilew</a><br></div></div></div>