<div dir="ltr">Okay - I hope to have fix tomorrow, will ask you for review :).</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 3, 2014 at 6:02 PM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That's fine too, that way you won't have to change up all the existing call sites of places using the current constructor. <br><br>As long as you think it won't take too long to fix, I'm fine leaving it as is until the fix goes in. I'm probably the most frequent builder of Windows, but I know there are at least a few others, so I don't want anyone to be broken for too long.<div class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">On Wed Dec 03 2014 at 5:58:30 PM Oleksiy Vyalov <<a href="mailto:ovyalov@google.com" target="_blank">ovyalov@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Or we may have 2 Pipe constructors - default creates anonymous pipe and constructor with parameter is for named pipe.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 3, 2014 at 5:41 PM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">What about passing a value to Pipe's constructor that is the name of the pipe? If it passes null, it's an anonymous pipe, if it passes non-null, it's a named pipe. The same model should be fine for Windows too.<div><div><br><br><div class="gmail_quote">On Wed Dec 03 2014 at 5:40:19 PM Oleksiy Vyalov <<a href="mailto:ovyalov@google.com" target="_blank">ovyalov@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sorry for breakage. <div><br></div><div>LLDB uses mkfifo to create a named pipe and for Windows there is just<b> if ( false ) </b>substitution - so, even with platform-independent read logic a named pipe creation should be abstracted as well.</div><div>Can we fix this problem in 2 steps?:</div><div> - Fix build with #ifdef _WIN32 #include <Winsock2.h> #else #include <sys/select.h> #endif (or even make ReadPortFromPipe available only for non-Windows configuration).</div><div> - Redesign create/read pipe API to make it compatible with Windows since <font face="arial, sans-serif"><span style="font-size:12.7272720336914px">PipePosix in its current state </span>represents<span style="font-size:12.7272720336914px"> anonymous pipe model only. </span></font></div></div><div class="gmail_extra"></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 3, 2014 at 5:03 PM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I took another look at the implementations. Pipe does not expose a method that will do what we want. I think the correct fix is to add an overload of Read() to PipePosix.h that specifies a timeout. In the overloaded version, you call this select. I will implement the equivalent overload on Windows so that everything will work.<br><div><br></div><div>Thoughts?</div><div><div><br><div class="gmail_quote">On Wed Dec 03 2014 at 4:55:17 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This breaks the Windows build. For starters, Windows doesn't have sys/select.h. Secondly, although Windows doesn't currently use llgs, it will in the future. And select() on Windows does not work with Pipes.<br><div><br></div><div>Can we revert this change? We have lldb/Host/Pipe.h that should be used for pipes instead of File. On Windows this is implemented in such a way that it's possible to get non-blocking behavior without select, and on posix I believe it uses select.</div><br><div class="gmail_quote">On Wed Dec 03 2014 at 10:25:23 AM Oleksiy Vyalov <<a href="mailto:ovyalov@google.com" target="_blank">ovyalov@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ovyalov<br>
Date: Wed Dec 3 12:19:16 2014<br>
New Revision: 223251<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=223251&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>pr<u></u><u></u>oject?rev=223251&view=rev</a><br>
Log:<br>
Use timeout when reading debugserver's port from a named pipe.<br>
<br>
<a href="http://reviews.llvm.org/D6490" target="_blank">http://reviews.llvm.org/D6490</a><br>
<br>
<br>
Modified:<br>
lldb/trunk/source/Plugins/<u></u>Proc<u></u><u></u>ess/gdb-remote/<u></u>GDBRemoteCommun<u></u><u></u>ication.cpp<br>
<br>
Modified: lldb/trunk/source/Plugins/<u></u>Proc<u></u><u></u>ess/gdb-remote/<u></u>GDBRemoteCommun<u></u><u></u>ication.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=223251&r1=223250&r2=223251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>pr<u></u><u></u>oject/lldb/trunk/source/<u></u>Plugin<u></u><u></u>s/Process/gdb-remote/<u></u>GDBRemote<u></u><u></u>Communication.cpp?<u></u>rev=223251&<u></u>r<u></u>1=223250&r2=<u></u>223251&view=diff</a><br>
==============================<u></u><u></u><u></u>==============================<u></u><u></u><u></u>==================<br>
--- lldb/trunk/source/Plugins/<u></u>Proc<u></u><u></u>ess/gdb-remote/<u></u>GDBRemoteCommun<u></u><u></u>ication.cpp (original)<br>
+++ lldb/trunk/source/Plugins/<u></u>Proc<u></u><u></u>ess/gdb-remote/<u></u>GDBRemoteCommun<u></u><u></u>ication.cpp Wed Dec 3 12:19:16 2014<br>
@@ -13,6 +13,7 @@<br>
// C Includes<br>
#include <limits.h><br>
#include <string.h><br>
+#include <sys/select.h><br>
#include <sys/stat.h><br>
<br>
// C++ Includes<br>
@@ -42,6 +43,50 @@<br>
using namespace lldb;<br>
using namespace lldb_private;<br>
<br>
+namespace<br>
+{<br>
+<br>
+Error<br>
+ReadPortFromPipe (const char *const named_pipe_path, uint16_t& port, const int timeout_secs)<br>
+{<br>
+ File name_pipe_file;<br>
+ auto error = name_pipe_file.Open (named_pipe_path, File::eOpenOptionRead | File::eOpenOptionNonBlocking);<br>
+ if (error.Fail ())<br>
+ return error;<br>
+<br>
+ struct timeval tv = {timeout_secs, 0};<br>
+ const auto pipe_handle = name_pipe_file.<u></u>GetWaitableHand<u></u><u></u>le ();<br>
+ fd_set rfds;<br>
+ FD_ZERO(&rfds);<br>
+ FD_SET(pipe_handle, &rfds);<br>
+<br>
+ const auto retval = ::select (pipe_handle + 1, &rfds, NULL, NULL, &tv);<br>
+ if (retval == -1)<br>
+ {<br>
+ error.SetErrorToErrno ();<br>
+ return error;<br>
+ }<br>
+ if (retval == 0)<br>
+ {<br>
+ error.SetErrorString ("timeout exceeded");<br>
+ return error;<br>
+ }<br>
+<br>
+ char port_cstr[256];<br>
+ port_cstr[0] = '\0';<br>
+ size_t num_bytes = sizeof(port_cstr);<br>
+ error = name_pipe_file.Read (port_cstr, num_bytes);<br>
+<br>
+ if (error.Success ())<br>
+ {<br>
+ assert (num_bytes > 0 && port_cstr[num_bytes-1] == '\0');<br>
+ port = Args::StringToUInt32 (port_cstr, 0);<br>
+ }<br>
+ return error;<br>
+}<br>
+<br>
+}<br>
+<br>
GDBRemoteCommunication::<u></u>Histo<u></u><u></u>ry::History (uint32_t size) :<br>
m_packets(),<br>
m_curr_idx (0),<br>
@@ -872,25 +917,23 @@ GDBRemoteCommunication::<u></u>StartD<u></u><u></u>ebugserver<br>
launch_info.<u></u>AppendSuppressFil<u></u><u></u>eAction (STDIN_FILENO, true, false);<br>
launch_info.<u></u>AppendSuppressFil<u></u><u></u>eAction (STDOUT_FILENO, false, true);<br>
launch_info.<u></u>AppendSuppressFil<u></u><u></u>eAction (STDERR_FILENO, false, true);<br>
-<br>
+<br>
error = Host::LaunchProcess(launch_<u></u>inf<u></u><u></u>o);<br>
-<br>
+<br>
if (error.Success() && launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID)<br>
{<br>
if (named_pipe_path[0])<br>
{<br>
- File name_pipe_file;<br>
- error = name_pipe_file.Open(named_<u></u>pipe<u></u><u></u>_path, File::eOpenOptionRead);<br>
+ error = ReadPortFromPipe(named_pipe_<u></u>pa<u></u><u></u>th, out_port, 10);<br>
if (error.Success())<br>
{<br>
- char port_cstr[256];<br>
- port_cstr[0] = '\0';<br>
- size_t num_bytes = sizeof(port_cstr);<br>
- error = name_pipe_file.Read(port_cstr, num_bytes);<br>
- assert (error.Success());<br>
- assert (num_bytes > 0 && port_cstr[num_bytes-1] == '\0');<br>
- out_port = Args::StringToUInt32(port_<u></u>cstr<u></u><u></u>, 0);<br>
- name_pipe_file.Close();<br>
+ if (log)<br>
+ log->Printf("<u></u>GDBRemoteCommunic<u></u><u></u>ation::%s() debugserver listens %u port", __FUNCTION__, out_port);<br>
+ }<br>
+ else<br>
+ {<br>
+ if (log)<br>
+ log->Printf("<u></u>GDBRemoteCommunic<u></u><u></u>ation::%s() failed to read a port value from named pipe %s: %s", __FUNCTION__, named_pipe_path, error.AsCString());<br>
}<br>
FileSystem::Unlink(named_<u></u>pipe<u></u>_<u></u>path);<br>
}<br>
<br>
<br>
______________________________<u></u><u></u><u></u>_________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailm<u></u><u></u>an/listinfo/lldb-commits</a><br>
</blockquote></div></blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div><div class="gmail_extra">-- <br><div><div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px"> <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;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px"> <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 class="gmail_signature"><div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;background-color:rgb(255,255,255);border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px"> <a href="mailto:ovyalov@google.com" target="_blank">ovyalov<font color="#1155cc">@google.com</font></a></span></div></div>
</div>