[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:40:18 PST 2014
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/20141203/2edc0810/attachment.html>
More information about the lldb-commits
mailing list