[Lldb-commits] [lldb] r223251 - Use timeout when reading debugserver's port from a named pipe.
Zachary Turner
zturner at google.com
Wed Dec 3 17:41:51 PST 2014
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.
On Wed Dec 03 2014 at 5:40:19 PM Oleksiy Vyalov <ovyalov at google.com> wrote:
> Sorry for breakage.
>
> LLDB uses mkfifo to create a named pipe and for Windows there is just* if
> ( false ) *substitution - so, even with platform-independent read logic a
> named pipe creation should be abstracted as well.
> Can we fix this problem in 2 steps?:
> - Fix build with #ifdef _WIN32 #include <Winsock2.h> #else #include
> <sys/select.h> #endif (or even make ReadPortFromPipe available only for
> non-Windows configuration).
> - Redesign create/read pipe API to make it compatible with Windows since PipePosix
> in its current state represents anonymous pipe model only.
>
> On Wed, Dec 3, 2014 at 5:03 PM, Zachary Turner <zturner at google.com> wrote:
>
>> 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
>>>>
>>>
>
>
> --
> Oleksiy Vyalov | Software Engineer | ovyalov at google.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141204/3607d5e4/attachment.html>
More information about the lldb-commits
mailing list