[Lldb-commits] [lldb] r223251 - Use timeout when reading debugserver's port from a named pipe.

[Zachary Turner]

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 <zturner at google.com> 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 <ovyalov at google.com>
> 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
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141204/90bace95/attachment.html>