[lldb-dev] Patch for garbage characters on quit

[Malea, Daniel]

Ah, didn't see this earlier, but looks ok to me. I can confirm I've also seen the garbage chars problem recently.

BTW, in the future it's probably best to send patches to LLDB-commits (rather than dev).. I pay more attention to that, although I try to watch both lists.

Dan

On 2013-05-21, at 5:59 PM, "Mike Sartain" <mikesart at valvesoftware.com<mailto:mikesart at valvesoftware.com>> wrote:

Just pinging on this one to see if it's ok?

Thanks much.

________________________________
From: lldb-dev-bounces at cs.uiuc.edu<mailto:lldb-dev-bounces at cs.uiuc.edu> [lldb-dev-bounces at cs.uiuc.edu<mailto:lldb-dev-bounces at cs.uiuc.edu>] on behalf of Matthew Sorrels [sorrels.m at gmail.com<mailto:sorrels.m at gmail.com>]
Sent: Friday, May 17, 2013 2:39 PM
To: lldb-dev at cs.uiuc.edu<mailto:lldb-dev at cs.uiuc.edu>
Subject: [lldb-dev] Patch for garbage characters on quit

It's possible others aren't seeing this but on Ubuntu 12.04 64 bits, when I exit lldb I get a few randomish garbage characters spewed to the terminal.  It appears the problem is the shutdown of master_out_comm in tools/driver/Driver.cpp around line 1591.  But I'm not sure I know enough about how this works to say what I'm doing to fix it is the best solution.  Other code for handling a Communication clear appears to shut them down by doing the following:

    SetReadThreadBytesReceivedCallback (NULL, NULL);
    Disconnect (NULL);
    StopReadThread (NULL);

Using this sequence on the master_out_comm seems to stop it from spewing random characters while quiting.  Just stoping the read thread wasn't a good solution, since it caused a huge pause.  Had to set the callback to NULL and disconnect first.

It also seemed kind of odd to me that you are closing the editline_output_pty before shutting down master_out_comm and out_comm_2 (which both were passed the editline file descriptor).  So I moved that CloseMasterFileDescriptor down after shutting down master_out_comm and out_comm_2.  Not sure this really needed to be done(didn't make any visible difference), but it felt right.

Here's the patch -- it seems to work for me (but I have no Mac to test it on), it's very possible I'm completely misunderstanding how this should work.  I'm still trying to get my feet wet on this code base.  Thanks,



Index: tools/driver/Driver.cpp
===================================================================
--- tools/driver/Driver.cpp    (revision 182137)
+++ tools/driver/Driver.cpp    (working copy)
@@ -1587,9 +1587,16 @@
                 }
             }

-            editline_output_pty.CloseMasterFileDescriptor();
+
+            master_out_comm.SetReadThreadBytesReceivedCallback(NULL, NULL);
             master_out_comm.Disconnect();
+            master_out_comm.ReadThreadStop();
+
+            out_comm_2.SetReadThreadBytesReceivedCallback(NULL, NULL);
             out_comm_2.Disconnect();
+            out_comm_2.ReadThreadStop();
+
+            editline_output_pty.CloseMasterFileDescriptor();
             reset_stdin_termios();
             fclose (stdin);


_______________________________________________
lldb-dev mailing list
lldb-dev at cs.uiuc.edu<mailto:lldb-dev at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev