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.
Thoughts? On Wed Dec 03 2014 at 4:55:17 PM Zachary Turner <[email protected]> wrote: > 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. > > 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. > > On Wed Dec 03 2014 at 10:25:23 AM Oleksiy Vyalov <[email protected]> > wrote: > >> Author: ovyalov >> Date: Wed Dec 3 12:19:16 2014 >> New Revision: 223251 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=223251&view=rev >> Log: >> Use timeout when reading debugserver's port from a named pipe. >> >> http://reviews.llvm.org/D6490 >> >> >> Modified: >> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommun >> ication.cpp >> >> Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommun >> ication.cpp >> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugin >> s/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=223251&r1=223250&r2= >> 223251&view=diff >> ============================================================ >> ================== >> --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp >> (original) >> +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp >> Wed Dec 3 12:19:16 2014 >> @@ -13,6 +13,7 @@ >> // C Includes >> #include <limits.h> >> #include <string.h> >> +#include <sys/select.h> >> #include <sys/stat.h> >> >> // C++ Includes >> @@ -42,6 +43,50 @@ >> using namespace lldb; >> using namespace lldb_private; >> >> +namespace >> +{ >> + >> +Error >> +ReadPortFromPipe (const char *const named_pipe_path, uint16_t& port, >> const int timeout_secs) >> +{ >> + File name_pipe_file; >> + auto error = name_pipe_file.Open (named_pipe_path, >> File::eOpenOptionRead | File::eOpenOptionNonBlocking); >> + if (error.Fail ()) >> + return error; >> + >> + struct timeval tv = {timeout_secs, 0}; >> + const auto pipe_handle = name_pipe_file.GetWaitableHandle (); >> + fd_set rfds; >> + FD_ZERO(&rfds); >> + FD_SET(pipe_handle, &rfds); >> + >> + const auto retval = ::select (pipe_handle + 1, &rfds, NULL, NULL, >> &tv); >> + if (retval == -1) >> + { >> + error.SetErrorToErrno (); >> + return error; >> + } >> + if (retval == 0) >> + { >> + error.SetErrorString ("timeout exceeded"); >> + return error; >> + } >> + >> + char port_cstr[256]; >> + port_cstr[0] = '\0'; >> + size_t num_bytes = sizeof(port_cstr); >> + error = name_pipe_file.Read (port_cstr, num_bytes); >> + >> + if (error.Success ()) >> + { >> + assert (num_bytes > 0 && port_cstr[num_bytes-1] == '\0'); >> + port = Args::StringToUInt32 (port_cstr, 0); >> + } >> + return error; >> +} >> + >> +} >> + >> GDBRemoteCommunication::History::History (uint32_t size) : >> m_packets(), >> m_curr_idx (0), >> @@ -872,25 +917,23 @@ GDBRemoteCommunication::StartDebugserver >> launch_info.AppendSuppressFileAction (STDIN_FILENO, true, >> false); >> launch_info.AppendSuppressFileAction (STDOUT_FILENO, false, >> true); >> launch_info.AppendSuppressFileAction (STDERR_FILENO, false, >> true); >> - >> + >> error = Host::LaunchProcess(launch_info); >> - >> + >> if (error.Success() && launch_info.GetProcessID() != >> LLDB_INVALID_PROCESS_ID) >> { >> if (named_pipe_path[0]) >> { >> - File name_pipe_file; >> - error = name_pipe_file.Open(named_pipe_path, >> File::eOpenOptionRead); >> + error = ReadPortFromPipe(named_pipe_path, out_port, 10); >> if (error.Success()) >> { >> - char port_cstr[256]; >> - port_cstr[0] = '\0'; >> - size_t num_bytes = sizeof(port_cstr); >> - error = name_pipe_file.Read(port_cstr, num_bytes); >> - assert (error.Success()); >> - assert (num_bytes > 0 && port_cstr[num_bytes-1] == >> '\0'); >> - out_port = Args::StringToUInt32(port_cstr, 0); >> - name_pipe_file.Close(); >> + if (log) >> + log->Printf("GDBRemoteCommunication::%s() >> debugserver listens %u port", __FUNCTION__, out_port); >> + } >> + else >> + { >> + if (log) >> + log->Printf("GDBRemoteCommunication::%s() >> failed to read a port value from named pipe %s: %s", __FUNCTION__, >> named_pipe_path, error.AsCString()); >> } >> FileSystem::Unlink(named_pipe_path); >> } >> >> >> _______________________________________________ >> lldb-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits >> >
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
