From 32720b51e22300b66d791755e3bb09821741cd09 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Sat, 14 Jan 2012 20:47:38 +0000 Subject: [PATCH] Fixed two double "int close(int fd)" issues found by our file descriptor interposing library on darwin: The first is in SBDebugger::SetInputFileHandle (FILE *file, bool transfer_ownership) where we would give our FILE * to a lldb_private::File object member variable and tell it that it owned the file descriptor if "transfer_ownership" was true, and then we would also give it to the communication plug-in that waits for stdin to come in and tell it that it owned the FILE *. They would both try and close the file. The seconds was when we use a file descriptor through ConnectionFileDescriptor where someone else is creating a connection with ConnectionFileDescriptor and a URL like: "fd://123". We were always taking ownwership of the fd 123, when we shouldn't be. There is a TODO in the comments that says we should allow URL options to be passed to be able to specify this later (something like: "fd://123?transer_ownership=1"), but we can get to this later. llvm-svn: 148201 --- lldb/source/Core/ConnectionFileDescriptor.cpp | 11 ++++++++++- lldb/source/Core/Debugger.cpp | 5 ++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lldb/source/Core/ConnectionFileDescriptor.cpp b/lldb/source/Core/ConnectionFileDescriptor.cpp index f83d9994c350..fec860a7583f 100644 --- a/lldb/source/Core/ConnectionFileDescriptor.cpp +++ b/lldb/source/Core/ConnectionFileDescriptor.cpp @@ -174,7 +174,16 @@ ConnectionFileDescriptor::Connect (const char *s, Error *error_ptr) bool is_socket = GetSocketOption (m_fd_send, SOL_SOCKET, SO_REUSEADDR, resuse) == 0; if (is_socket) m_fd_send_type = m_fd_recv_type = eFDTypeSocket; - m_should_close_fd = true; + // Don't take ownership of a file descriptor that gets passed + // to us since someone else opened the file descriptor and + // handed it to us. + // TODO: Since are using a URL to open connection we should + // eventually parse options using the web standard where we + // have "fd://123?opt1=value;opt2=value" and we can have an + // option be "owns=1" or "owns=0" or something like this to + // allow us to specify this. For now, we assume we must + // assume we don't own it. + m_should_close_fd = false; return eConnectionStatusSuccess; } } diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index f28c261b4145..72449efb4c37 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -410,7 +410,10 @@ Debugger::SetInputFileHandle (FILE *fh, bool tranfer_ownership) // Disconnect from any old connection if we had one m_input_comm.Disconnect (); - m_input_comm.SetConnection (new ConnectionFileDescriptor (in_file.GetDescriptor(), true)); + // Pass false as the second argument to ConnectionFileDescriptor below because + // our "in_file" above will already take ownership if requested and we don't + // want to objects trying to own and close a file descriptor. + m_input_comm.SetConnection (new ConnectionFileDescriptor (in_file.GetDescriptor(), false)); m_input_comm.SetReadThreadBytesReceivedCallback (Debugger::DispatchInputCallback, this); Error error;