[Lldb-commits] [lldb] r223251 - Use timeout when reading debugserver's port from a named pipe.
Oleksiy Vyalov
ovyalov at google.com
Wed Dec 3 17:58:29 PST 2014
Or we may have 2 Pipe constructors - default creates anonymous pipe and
constructor with parameter is for named pipe.
On Wed, Dec 3, 2014 at 5:41 PM, Zachary Turner <zturner at google.com> wrote:
> 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
>>
>
--
Oleksiy Vyalov | Software Engineer | ovyalov at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141203/a9ca50b5/attachment.html>
More information about the lldb-commits
mailing list