Re: Reworked qt terminal is much faster – and fails to work on Mac

[Daniel J S. <dan...@ie...>]

On 02/11/2014 03:15 PM, Thomas Bleher wrote:
> * Ethan A Merritt<sf...@us...>  [2014-02-11 21:35]:
>> On Sunday, 09 February, 2014 16:57:36 sfeam wrote:
>>>> And indeed if I change the timeout:
>>>>
>>>> --- a/src/qtterminal/qt_term.cpp
>>>> +++ b/src/qtterminal/qt_term.cpp
>>>> @@ -251,7 +252,7
>>>>
>>>>          // The QLocalSocket::waitForConnected does not respect the
>>>> time out argument when the
>>>>          // gnuplot_qt application is not yet started. To wait for it,
>>>> we need to implement the timeout ourselves
>>>> -       QDateTime timeout = QDateTime::currentDateTime().addMSecs(1000);
>>>> +       QDateTime timeout = QDateTime::currentDateTime().addMSecs(10000);
>>>>          do
>>>>          {
>>>>                  qt->socket.connectToServer(server);
>>>>
>>>> it suddenly almost starts working. I'm saying almost because the first
>>>> plot doesn't work, but the second one does.
>>>
>>> Hence my suspicion that timeouts<  1000msec are not working.
>>> I suggest you do a global search and replace for timeouts and make
>>> sure they are all>1000 msec.
>>
>> I think I have uncovered part of the problem, but I do not understand
>> why it is failing.
>>
>> This loop:
>>          do
>>          {
>>                  qt->socket.connectToServer(server);
>>                  qt->socket.waitForConnected(200);
>>                  // TODO: yield CPU ?
>>          }
>> 	while(...)
>>
>> Does not work as intended.
>>
>> To see why, change it to this:
>> 	qDebug()<<  "qt_connectToServer "<<  server;
>>          do
>>          {
>>                  qt->socket.connectToServer(server);
>>                  if (!qt->socket.waitForConnected(TIMEOUT))
>>                          qDebug()<<  qt->socket.errorString();
>>          	usleep(50000);
>> 	}
>>          while(...)
>>
>> Regardless of the value of TIMEOUT, including -1 or blank,  the
>> waitForConnected returns immediately with an error:
>>           "QLocalSocket::connectToServer: Invalid name"
>
> [Note: I haven't looked at this in detail, so these are just my guesses]
>
> I think the error is probably a race condition:
> - gnuplot starts the external gnuplot_qt process
> - Immediately afterwards it tries to connect to the socket from
>    gnuplot_qt
> - Depending on scheduling, gnuplot_qt may have had a chance to run or
>    not. But in any case it needs to do some initialization first.
>
> So my guess is that the "Invalid name" error stems from the fact that
> during the first iteration of the loop, the named pipe (which is created
> by gnuplot_qt) is simply not there yet.

This is correct, but also how the software is designed.  It continues in 
a loop until it can find the pipe or timeout occurs.  I've printed out 
the server name on my linux box and there is a stream of:

.
.
.
Server:  "qtgnuplot13225"
Server:  "qtgnuplot13225"
Server:  "qtgnuplot13225"
Server:  "qtgnuplot13225"
gnuplot>

before the plot appears.  So it probably isn't too difficult for 1 
second to go by at the system level before an application is up and 
running.  So, I'd say make that timeout just a tad longer.

Notice that this is a recurrent function.  When the timeout does occur, 
eventually the logic gets to:

		if (connectToWidget)
		{
			qDebug() << "Could not connect to widget" << qt_option->Widget << ". 
Starting a QtGnuplotApplication";
			qt_option->Widget = QString();
			qt_connectToServer(qt->localServerName);
		}

so that calls this function a second time, but at the top of the 
function there is

	bool connectToWidget = (server != qt->localServerName);

So then what happens if there is a second failure to connect?  Well, 
then the logic becomes:

		else
		{
			qDebug() << "Could not connect gnuplot_qt" << qt_option->Widget << ". 
Starting a new one";
			execGnuplotQt();
			qt_connectToServer(qt->localServerName, false);
		}

and the argument is 'false' so will not try after the third time.

In any case, I suggest adding the sleep as Ethan has done.  There is no 
reason that should be running at 100%.  In fact, there is probably a 
preferred way to do this without polling loops.  I learned a little bit 
about Qt working on Octave and Qt has this paradigm of signals and slots 
and the developers suggest adhering to the concept otherwise can get 
kind of dodgy (not in this simple case...but cases where widget IDs are 
floating about).

signal:  something that a Qt object emits
slot:  the destination of the signal which can
        be of any number, e.g., five other
        objects could connect a slot to a signal

Anyhow, I don't have time to look at this right now, but if one looks at 
the documentation for a Qt socket:

http://qt-project.org/doc/qt-4.8/qlocalsocket.html#connectToServer

it indicates that a signal is emitted when the connection is complete 
and there is a signal emitted when there is an error.  So the proper 
thing to do is to first make connections to the socket sort of like the 
following ("success" and "failed" are custom member functions):

connect (createdsocket, SIGNAL (connected ()), watcher, SLOT (success ()));
connect (createdsocket, SIGNAL (error ()), watcher, SLOT (failed ()));

and then tell the socket to attempt to connect to the server:

createdsocket->connectToServer (name)

There is no need to check in a loop for anything.  Either the socket 
will successfully connect and emit "connected" at which point 
"success()" will get called or the socket will timeout and emit "error" 
at which point "failed()" will get called.

One can get very creative about connections made, the number of slots 
watching a signal, doing this dynamically, etc.  So instead of a 
recursive routine, it might be multiple connections, or dynamically 
reconnect/disconnect in the "failed()" slot.  Etc.

Dan