<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Thu, May 21, 2015 at 10:58 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">I feel like it should be the other way around. If LockFile is taking a reference then it is implicitly saying that someone else owns the underlying File handle. But who else cares about the contents of a lock file other than the LockFile object itself?<br><div><br></div></div></blockquote><div>It's not the case in current LockFile usage scenario but I can imagine following use case - somebody wants to synchronize IO operations on file's content:</div><div><ul><li>With multiple readers and awaiting writers</li><li>Single writer and awaiting readers</li></ul></div><div>After you locked a file with LockFile you can proceed on IO operations and be confident that these operations are synchronized - e.g., instead of zero-size lock file we could have used module's file as synchronization point and to lock a module file first before pouring data into it..</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div></div><div>Why not just change the LockFile constructor to not take a file descriptor but instead take the path? This is even better because now in LockFileWindows you don't even need to use an fd you can just store a HANDLE directly, and it gives you more flexibility with how you create the file because you can use windows-specific options such as FILE_FLAG_DELETE_ON_CLOSE (if you want that, anyway).</div><div><br></div><div>If you're concerned about the case where the Open operation fails and you want to handle that error, then don't let the constructor take anything, but instead you could make a LockFile::Open() method or a static LockFile &&LockFile::Create() method. (by returning r-value reference here you could make LockFile object moveable but non-copyable, which makes sense for something like a lock file.</div></div><div><div><br><div class="gmail_quote">On Thu, May 21, 2015 at 10:50 AM Oleksiy Vyalov <<a href="mailto:ovyalov@google.com" target="_blank">ovyalov@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">I'm somewhat concerned about following scenario:<div><br></div><div>File file_obj(...);</div><div><span style="font-size:12.8000001907349px">LockFile lock (</span>file_obj<span style="font-size:12.8000001907349px">.GetDescriptor ());</span><br></div><div><span style="font-size:12.8000001907349px">..</span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">My understanding, that on Windows if </span><span style="font-size:12.8000001907349px">LockFile duplicates original handle (i.e. </span><span style="font-size:12.8000001907349px">file.GetDescriptor ()</span><span style="font-size:12.8000001907349px">) only a duplicated handle will be allowed to call IO functions on this file within locked region, meanwhile </span>file_obj will be blocked on IO.</div><div>If <span style="font-size:12.8000001907349px">LockFile just holds a reference to original handle then </span>file_obj can proceed on IO operations in exclusive mode.<span style="font-size:12.8000001907349px"> </span></div><div><br></div><div><span style="font-size:12.8000001907349px">In order to make it clear that </span><span style="font-size:12.8000001907349px">LockFile references File we can make LockFile to take File& as constructor parameter instead of a raw handle.</span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 21, 2015 at 10:28 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Why does someone other than the LockFile object care about holding a raw handle? Shouldn't it just hold a reference/pointer to the LockFile? <br><div><div><div class="gmail_quote">On Thu, May 21, 2015 at 10:06 AM Oleksiy Vyalov <<a href="mailto:ovyalov@google.com" target="_blank">ovyalov@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Windows file lock is tied to a file handle - using handle duplication effectively applies locking on duplicated handle, not on original, so your original handle will be blocked on IO trying to read/write.<div>Maybe we can just eliminate CloseHandle call (<a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_llvm-2Dmirror_lldb_blob_master_source_Host_windows_LockFileWindows.cpp-23L50&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DDUMf06MYELAe1Nlv7KChiwJLLHbYha4jtK_AOiWqwQ&m=ROvwrDYgfjpuzDwq0gKRzlv2JTHNXjr97B3GDRbdOLk&s=KnFqeEOY6w4Ja5lOEvgx-wW8C9siuxcXcbUKj1mejC0&e=" target="_blank">https://github.com/llvm-mirror/lldb/blob/master/source/Host/windows/LockFileWindows.cpp#L50</a>) if file will be eventually closed by LockFileWindows caller.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 21, 2015 at 9:58 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I don't experience this crash but looking at the proposed fix it certainly seems like a plausible explanation. Seems to me like something should transfer ownership though rather than duplicating the handle<div><div><br><div class="gmail_quote">On Thu, May 21, 2015 at 9:52 AM Colin Riley <<a href="mailto:colin@codeplay.com" target="_blank">colin@codeplay.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
Windows 7 64, Debug, latest vs13.</div><div bgcolor="#FFFFFF" text="#000000"><br>
<br>
<div>On 21/05/2015 17:39, Oleksiy Vyalov
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Colin,
<div><br>
</div>
<div>could you give more context about crash - what build
configuration do you use (debug, release,..) and which OS?</div>
<div>I'm running this code on Windows 7 and haven't noticed any
failures.</div>
<div><br>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, May 21, 2015 at 8:57 AM, Colin
Riley <span dir="ltr"><<a href="mailto:colin@codeplay.com" target="_blank">colin@codeplay.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> <br>
Zachary, do you see this on windows at all? Tip for us
results in crashes when releasing file descriptors without
the below fix. <br>
<br>
Colin
<div>
<div><br>
<br>
<div>On 19/05/2015 12:52, Aidan Dodds wrote:<br>
</div>
</div>
</div>
<blockquote type="cite">
<div>
<div>Hi, <br>
<br>
We have been seeing a crash on windows when
connecting to an android target using lldb-server. <br>
I am not sure if this affects other platforms too. <br>
I think this was introduced with <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9056&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DDUMf06MYELAe1Nlv7KChiwJLLHbYha4jtK_AOiWqwQ&m=ROvwrDYgfjpuzDwq0gKRzlv2JTHNXjr97B3GDRbdOLk&s=zEUB9hl5-3dJpUGBI2vyZFa3I52nfs1GIW58IbieaJY&e=" target="_blank">http://reviews.llvm.org/D9056</a>.
<br>
<br>
I tracked the crash back to the workings of
ModuleCache::GetAndPut(). <br>
<br>
The crash seems to be due to a file descriptor being
released twice, once by the original "File
lock_file" and again by the "LockFile lock" who
share the same file descriptor. <br>
<br>
The file descriptor sharing happens because of this
line: <br>
ModuleCache.cpp @ 164 <br>
LockFile lock (lock_file.GetDescriptor ()); <br>
<br>
Both destructors attempt to release effectively the
same file descriptor. I was able to fix the crash
by duplicating the file handle in the lock file
constructor using _dup(). (patch attached) <br>
I wasn't sure if this was the right fix however. Has
anyone else seen this? <br>
Should "File lock_file" perhaps transfer its file
descriptor completely rather then share it? <br>
<br>
Thanks, <br>
Aidan <br>
<br>
<fieldset></fieldset>
<br>
</div>
</div>
<pre>_______________________________________________
lldb-dev mailing list
<a href="mailto:lldb-dev@cs.uiuc.edu" target="_blank">lldb-dev@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a>
</pre>
</blockquote>
<br>
<pre cols="72">--
- Colin Riley
Senior Director,
Parallel/Graphics Debugger Systems
Codeplay Software Ltd
45 York Place, Edinburgh, EH1 3HP
Tel: 0131 466 0503
Fax: 0131 557 6600
Website: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__www.codeplay.com&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DDUMf06MYELAe1Nlv7KChiwJLLHbYha4jtK_AOiWqwQ&m=ROvwrDYgfjpuzDwq0gKRzlv2JTHNXjr97B3GDRbdOLk&s=jvpSpfWzICTWCMqDDDYk2m-Mdgm2hvxkltXdPBw4DBs&e=" target="_blank">http://www.codeplay.com</a>
Twitter: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__twitter.com_codeplaysoft&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DDUMf06MYELAe1Nlv7KChiwJLLHbYha4jtK_AOiWqwQ&m=ROvwrDYgfjpuzDwq0gKRzlv2JTHNXjr97B3GDRbdOLk&s=HfgPUVkbftPZuAj9hsvP6cSHm5zx1Q_v0r8tSCC-4Yw&e=" target="_blank">https://twitter.com/codeplaysoft</a>
This email and any attachments may contain confidential and /or privileged information and is for use by the addressee only. If you are not the intended recipient, please notify Codeplay Software Ltd immediately and delete the message from your computer. You may not copy or forward it,or use or disclose its contents to any other person. Any views or other information in this message which do not relate to our business are not authorized by Codeplay software Ltd, nor does this message form part of any contract unless so stated.
As internet communications are capable of data corruption Codeplay Software Ltd does not accept any responsibility for any changes made to this message after it was sent. Please note that Codeplay Software Ltd does not accept any liability or responsibility for viruses and it is your responsibility to scan any attachments.
Company registered in England and Wales, number: 04567874
Registered office: 81 Linkfield Street, Redhill RH1 6BY </pre>
</div>
</blockquote>
</div>
<br>
<br clear="all">
<div><br>
</div>
-- <br>
<div>
<div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)">Oleksiy
Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> Software
Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> <a href="mailto:ovyalov@google.com" target="_blank">ovyalov<font color="#1155cc">@google.com</font></a></span></div>
</div>
</div>
</blockquote>
<br>
<pre cols="72">--
- Colin Riley
Senior Director,
Parallel/Graphics Debugger Systems
Codeplay Software Ltd
45 York Place, Edinburgh, EH1 3HP
Tel: 0131 466 0503
Fax: 0131 557 6600
Website: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__www.codeplay.com&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DDUMf06MYELAe1Nlv7KChiwJLLHbYha4jtK_AOiWqwQ&m=ROvwrDYgfjpuzDwq0gKRzlv2JTHNXjr97B3GDRbdOLk&s=jvpSpfWzICTWCMqDDDYk2m-Mdgm2hvxkltXdPBw4DBs&e=" target="_blank">http://www.codeplay.com</a>
Twitter: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__twitter.com_codeplaysoft&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DDUMf06MYELAe1Nlv7KChiwJLLHbYha4jtK_AOiWqwQ&m=ROvwrDYgfjpuzDwq0gKRzlv2JTHNXjr97B3GDRbdOLk&s=HfgPUVkbftPZuAj9hsvP6cSHm5zx1Q_v0r8tSCC-4Yw&e=" target="_blank">https://twitter.com/codeplaysoft</a>
This email and any attachments may contain confidential and /or privileged information and is for use by the addressee only. If you are not the intended recipient, please notify Codeplay Software Ltd immediately and delete the message from your computer. You may not copy or forward it,or use or disclose its contents to any other person. Any views or other information in this message which do not relate to our business are not authorized by Codeplay software Ltd, nor does this message form part of any contract unless so stated.
As internet communications are capable of data corruption Codeplay Software Ltd does not accept any responsibility for any changes made to this message after it was sent. Please note that Codeplay Software Ltd does not accept any liability or responsibility for viruses and it is your responsibility to scan any attachments.
Company registered in England and Wales, number: 04567874
Registered office: 81 Linkfield Street, Redhill RH1 6BY </pre>
</div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> <a href="mailto:ovyalov@google.com" target="_blank">ovyalov<font color="#1155cc">@google.com</font></a></span></div></div>
</div>
</blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> <a href="mailto:ovyalov@google.com" target="_blank">ovyalov<font color="#1155cc">@google.com</font></a></span></div></div>
</div>
</blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> <a href="mailto:ovyalov@google.com" target="_blank">ovyalov<font color="#1155cc">@google.com</font></a></span></div></div>
</div></div>