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 <[email protected]> wrote: > Thank you > On 3 Dec 2014 15:50, "Abid, Hafiz" <[email protected]> wrote: > >> Thanks for catching this. I have removed the extra changes in r223227. >> >> >> >> Thanks, >> >> Abid >> >> >> >> *From:* Ilia K [mailto:[email protected]] >> *Sent:* 03 December 2014 12:40 >> *To:* Abid, Hafiz >> *Cc:* [email protected] >> *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" <[email protected]> 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:[email protected]] >> *Sent:* 03 December 2014 10:59 >> *To:* Abid, Hafiz >> *Cc:* [email protected] >> *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 <[email protected]> >> 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 [email protected]. >> >> >> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits >> >> >> >
lldbmi_keep_icanon_mode.patch
Description: Binary data
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
