Hi Ilia, Thanks for the patch. It looks much cleaner. I was also not sure how tcgetattr type functions were helping with the original fix. Regarding your patch, I only have one comment. It is now returning false when ioctl fails. It can result in lldb-mi quitting. Previously this function was always returning true. I don’t think this is a big deal though.
Regarding your question about setbuf, I thought that setting it to null makes ioctl type function to return when some input is available and not wait for the new line character. I may be wrong here too. Can you try on OSX after removing the setbuf call. Does that change the behaviour in any way. Because I cannot see any noticeable difference on Linux. Regards, Abid From: Ilia K [mailto:[email protected]] Sent: 03 December 2014 16:17 To: Abid, Hafiz Cc: [email protected] Subject: Re: [Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run. 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]<mailto:[email protected]>> wrote: Thank you On 3 Dec 2014 15:50, "Abid, Hafiz" <[email protected]<mailto:[email protected]>> wrote: Thanks for catching this. I have removed the extra changes in r223227. Thanks, Abid From: Ilia K [mailto:[email protected]<mailto:[email protected]>] Sent: 03 December 2014 12:40 To: Abid, Hafiz Cc: [email protected]<mailto:[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]<mailto:[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]<mailto:[email protected]>] Sent: 03 December 2014 10:59 To: Abid, Hafiz Cc: [email protected]<mailto:[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]<mailto:[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]<mailto:[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]<mailto:[email protected]> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
