ovyalov added a comment.
In http://reviews.llvm.org/D13881#270878, @labath wrote:
> I *think* it looks good, but I find it quite hard to review a change of this
> size.
> A wise man once said "if your commit description contains bullet points, you
> are doing too much". I think it would be better to split up changes like this
> in the future.
>
> > Make PlatformRemoteGDBServer to support retry-based connect - both for
> > ConnectRemote and debug server connect. Since in case of domain sockets
> > lldb-server doesn't write port into named pipe we cannot use it as a
> > synchronization point - so it's possible that qLaunchGDBServer will be
> > received by host before debug server is up and listens on socket.
>
>
> I would like to preserve the invariant that by the time a reply to
> qLaunchGDBServer is received, the server is up and ready to serve
> connections. How about we keep this file synchronization protocol even when
> using domain sockets? The server could write nothing (or the socket path, or
> whatever) to the file when it is up and ready. That way, we don't have to
> increase the number of places that do those silly wait-and-retry loops. What
> do you think?
I was thinking about sending a full connect URL(instead of port) via pipe from
debug server to platform/lldb host - so, it can be less transport type
dependent. But there are some problems here - e.g., we cannot compose TCP
connect URL properly on server side because only client knows a proper server's
host address (if server is sitting behind proxies/firewalls/...). So, ended up
with extending Acceptor with GetLocalSocketId method which returns a string
either with TCP port or socket path so we can wait until debug server is up in
either case.
================
Comment at: source/Plugins/Platform/Android/AdbClient.cpp:151
@@ +150,3 @@
+ char message[PATH_MAX];
+ snprintf (message, sizeof (message), "forward:tcp:%d;localfilesystem:%s",
local_port, remote_socket_name);
+
----------------
I suspect adb fails in such situation.
Let me verify it once lldb-server/Acceptor will support protocol-based URL like
unix:///tmp/platform.sock. Now it's somewhat problematic because we cannot pass
colon as a part of
URL - if colon is found it's treated as $host:$port pattern and TCPSocket is
created.
================
Comment at: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h:227-228
@@ -226,7 +226,4 @@
lldb::UnixSignalsSP m_remote_signals_sp;
- // Launch the lldb-gdbserver on the remote host and return the port it is
listening on or 0 on
- // failure. Subclasses should override this method if they want to do
extra actions before or
- // after launching the lldb-gdbserver.
- virtual uint16_t
- LaunchGDBserverAndGetPort (lldb::pid_t &pid);
+ // Launch the debug server on the remote host - caller connects to launched
+ // debug server using connect_url.
----------------
Done.
http://reviews.llvm.org/D13881
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
