[Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.

Ilia K ki.stfu at gmail.com
Wed Dec 3 08:16:30 PST 2014


Hi Abid,

Sorry for remarks, but there is a one problem with "ioctl" patch: it turns
off canonical mode so we can't modify commands on input line correctly :(.

Also I added error checks and moved "setbuf(stdin, NULL)" into Initialize
function to avoid ugly in-place initialization using static variable (but,
if be honest I don't know why we need to disable bufferization before
"ioctl" call).

I prepared new patch. Review it please.

Thanks,
Ilia


On Wed, Dec 3, 2014 at 3:52 PM, Ilia K <ki.stfu at gmail.com> wrote:

> Thank you
> On 3 Dec 2014 15:50, "Abid, Hafiz" <Hafiz_Abid at mentor.com> wrote:
>
>>  Thanks for catching this. I have removed the extra changes in r223227.
>>
>>
>>
>> Thanks,
>>
>> Abid
>>
>>
>>
>> *From:* Ilia K [mailto:ki.stfu at gmail.com]
>> *Sent:* 03 December 2014 12:40
>> *To:* Abid, Hafiz
>> *Cc:* lldb-commits at cs.uiuc.edu
>> *Subject:* RE: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while
>> executing -exec-run.
>>
>>
>>
>> Yes. "select" patch requires extra handling on exit.
>>
>> Thanks,
>> Ilia
>>
>> On 3 Dec 2014 15:37, "Abid, Hafiz" <Hafiz_Abid at mentor.com> wrote:
>>
>> You mean that just the change in ‘InputAvailable’ was needed? Rest of the
>> changes were required with ‘select’.
>>
>>
>>
>> Regards,
>>
>> Abid
>>
>>
>>
>> *From:* Ilia K [mailto:ki.stfu at gmail.com]
>> *Sent:* 03 December 2014 10:59
>> *To:* Abid, Hafiz
>> *Cc:* lldb-commits at cs.uiuc.edu
>> *Subject:* Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while
>> executing -exec-run.
>>
>>
>>
>> Hello Abid,
>>
>>
>>
>> Thanks for commit :)
>>
>> But "ioctl" patch requires less changes than "select" version of this
>> patch. Could you commit another patch to revert all unnecessary changes?
>>
>>
>>
>> Thanks,
>>
>> Ilia
>>
>>
>>
>> On Wed, Dec 3, 2014 at 1:23 PM, Hafiz Abid Qadeer <hafiz_abid at mentor.com>
>> wrote:
>>
>> Author: abidh
>> Date: Wed Dec  3 04:23:06 2014
>> New Revision: 223222
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=223222&view=rev
>> Log:
>> Fix a hang on OSX while executing -exec-run.
>> Now we wait for input to become available before blocking in fgets.
>> More details on problem can be found in
>>
>> http://lists.cs.uiuc.edu/pipermail/lldb-commits/Week-of-Mon-20141201/014290.html
>>
>> Patch from dawn at burble.org.
>>
>>
>> Modified:
>>     lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp
>>     lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h
>>     lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp
>>     lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h
>>     lldb/trunk/tools/lldb-mi/MIDriver.cpp
>>
>> Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp?rev=223222&r1=223221&r2=223222&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp (original)
>> +++ lldb/trunk/tools/lldb-mi/MICmnStreamStdin.cpp Wed Dec  3 04:23:06 2014
>> @@ -434,3 +434,16 @@ CMICmnStreamStdin::SetOSStdinHandler(IOS
>>
>>      return MIstatus::success;
>>  }
>> +
>> +//++
>> ------------------------------------------------------------------------------------
>> +// Details: Do some actions before exiting.
>> +// Type:    Method.
>> +// Args:    None.
>> +// Return:  None.
>> +// Throws:  None.
>> +//--
>> +void
>> +CMICmnStreamStdin::OnExitHandler(void)
>> +{
>> +    m_pStdinReadHandler->InterruptReadLine();
>> +}
>>
>> Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h?rev=223222&r1=223221&r2=223222&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h (original)
>> +++ lldb/trunk/tools/lldb-mi/MICmnStreamStdin.h Wed Dec  3 04:23:06 2014
>> @@ -66,6 +66,7 @@ class CMICmnStreamStdin : public CMICmnB
>>        public:
>>          virtual bool InputAvailable(bool &vwbAvail) = 0;
>>          virtual const MIchar *ReadLine(CMIUtilString &vwErrMsg) = 0;
>> +        virtual void InterruptReadLine(void){};
>>
>>          /* dtor */ virtual ~IOSStdinHandler(void){};
>>      };
>> @@ -82,6 +83,7 @@ class CMICmnStreamStdin : public CMICmnB
>>      void SetCtrlCHit(void);
>>      bool SetVisitor(IStreamStdin &vrVisitor);
>>      bool SetOSStdinHandler(IOSStdinHandler &vrHandler);
>> +    void OnExitHandler(void);
>>
>>      // Overridden:
>>    public:
>>
>> Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp?rev=223222&r1=223221&r2=223222&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp (original)
>> +++ lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.cpp Wed Dec  3
>> 04:23:06 2014
>> @@ -22,7 +22,9 @@
>>  // Third Party Headers:
>>  #if !defined(_MSC_VER)
>>  #include <sys/select.h>
>> +#include <unistd.h>
>>  #include <termios.h>
>> +#include <sys/ioctl.h>
>>  #endif              // !defined( _MSC_VER )
>>  #include <string.h> // For std::strerror()
>>
>> @@ -153,30 +155,29 @@ CMICmnStreamStdinLinux::Shutdown(void)
>>  bool
>>  CMICmnStreamStdinLinux::InputAvailable(bool &vwbAvail)
>>  {
>> -    /* AD: Not used ATM but could come in handy just in case we need to
>> do
>> -           this, poll for input
>> -
>> -            static const int STDIN = 0;
>> -        static bool bInitialized = false;
>> -
>> -        if( !bInitialized )
>> -            {
>> -            // Use termios to turn off line buffering
>> -            ::termios term;
>> -            ::tcgetattr( STDIN, &term );
>> -            ::term.c_lflag &= ~ICANON;
>> -            ::tcsetattr( STDIN, TCSANOW, &term );
>> -            ::setbuf( stdin, NULL );
>> -            bInitialized = true;
>> -        }
>> -
>> -        int nBytesWaiting;
>> -        ::ioctl( STDIN, FIONREAD, &nBytesWaiting );
>> -        vwbAvail = (nBytesWaiting > 0);
>> -
>> -            return MIstatus::success;
>> -    */
>> -
>> +#if !defined(_WIN32)
>> +    // The code below is needed on OSX where lldb-mi hangs when doing
>> -exec-run.
>> +    // The hang seems to come from calling fgets and fileno from
>> different thread.
>> +    // Although this problem was not observed on Linux.
>> +    // A solution based on 'select' was also proposed but it seems to
>> slow things down
>> +    // a lot.
>> +    static bool bInitialized = false;
>> +
>> +    if (!bInitialized)
>> +    {
>> +        // Use termios to turn off line buffering
>> +        ::termios term;
>> +        ::tcgetattr(STDIN_FILENO, &term);
>> +        term.c_lflag &= ~ICANON;
>> +        ::tcsetattr(STDIN_FILENO, TCSANOW, &term);
>> +        ::setbuf(stdin, NULL);
>> +        bInitialized = true;
>> +    }
>> +
>> +    int nBytesWaiting;
>> +    ::ioctl(STDIN_FILENO, FIONREAD, &nBytesWaiting);
>> +    vwbAvail = (nBytesWaiting > 0);
>> +#endif
>>      return MIstatus::success;
>>  }
>>
>> @@ -213,3 +214,16 @@ CMICmnStreamStdinLinux::ReadLine(CMIUtil
>>
>>      return pText;
>>  }
>> +
>> +//++
>> ------------------------------------------------------------------------------------
>> +// Details: Interrupt current and prevent new ReadLine operations.
>> +// Type:    Method.
>> +// Args:    None.
>> +// Return:  None.
>> +// Throws:  None.
>> +//--
>> +void
>> +CMICmnStreamStdinLinux::InterruptReadLine(void)
>> +{
>> +    fclose(stdin);
>> +}
>>
>> Modified: lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h?rev=223222&r1=223221&r2=223222&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h (original)
>> +++ lldb/trunk/tools/lldb-mi/MICmnStreamStdinLinux.h Wed Dec  3 04:23:06
>> 2014
>> @@ -51,6 +51,7 @@ class CMICmnStreamStdinLinux : public CM
>>      // From CMICmnStreamStdin::IOSpecificReadStreamStdin
>>      virtual bool InputAvailable(bool &vwbAvail);
>>      virtual const MIchar *ReadLine(CMIUtilString &vwErrMsg);
>> +    virtual void InterruptReadLine(void);
>>
>>      // Methods:
>>    private:
>>
>> Modified: lldb/trunk/tools/lldb-mi/MIDriver.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIDriver.cpp?rev=223222&r1=223221&r2=223222&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/tools/lldb-mi/MIDriver.cpp (original)
>> +++ lldb/trunk/tools/lldb-mi/MIDriver.cpp Wed Dec  3 04:23:06 2014
>> @@ -1075,6 +1075,7 @@ CMIDriver::SetExitApplicationFlag(const
>>      {
>>          CMIUtilThreadLock lock(m_threadMutex);
>>          m_bExitApp = true;
>> +        m_rStdin.OnExitHandler();
>>          return;
>>      }
>>
>> @@ -1089,6 +1090,7 @@ CMIDriver::SetExitApplicationFlag(const
>>      }
>>
>>      m_bExitApp = true;
>> +    m_rStdin.OnExitHandler();
>>  }
>>
>>  //++
>> ------------------------------------------------------------------------------------
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141203/4cf04edb/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lldbmi_keep_icanon_mode.patch
Type: application/octet-stream
Size: 1490 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141203/4cf04edb/attachment.obj>


More information about the lldb-commits mailing list