[Lldb-commits] [lldb] r223222 - Fix a hang on OSX while executing -exec-run.
Ilia K
ki.stfu at gmail.com
Thu Dec 4 02:40:58 PST 2014
Hi Abid,
> It is now returning false when ioctl fails
I think that it's right, because status is unavailable.
> 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 realized that setbuf is not needed. As you said, it makes ioctl type
function to return when some input is available and not wait for the new
line character, but in canonical mode, you don't get control back until the
new line character had typed.
> Can you try on OSX after removing the setbuf call
I tried to remove setbuf() yesterday and it didn't affect, so I think it is
not needed. Could you remove code related to setbuf() from my patch
yourself?
Thanks,
Ilia
On Thu, Dec 4, 2014 at 1:17 PM, Abid, Hafiz <Hafiz_Abid at mentor.com> wrote:
> 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:ki.stfu at gmail.com]
> *Sent:* 03 December 2014 16:17
>
> *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.
>
>
>
> 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/20141204/fce8e7b5/attachment.html>
More information about the lldb-commits
mailing list