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

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

On 02/12/2014 01:25 AM, sfeam wrote:
> On Tuesday, 11 February 2014 10:15:24 PM Thomas Bleher wrote:
>> * Ethan A Merritt<sf...@us...>  [2014-02-11 21:35]:
>>
>>> 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.
>
> That makes sense, but the first attempt to connect _always_ fails,
> even if I explicitly wait for several seconds before invoking waitForConnected().
> So I think it's not just a timing race.  There's something that doesn't
> get initialized until the first connection attempt. So the first attempt fails
> but then the next attempt succeeds.
>
> I have gone ahead and placed a variant of that modification into CVS.
> I think the worst it can do is hang a session that was going to fail anyhow.
> We can still fix the source of the initial failure if we ever find it.
>
>> I also looked through the recent changes to the gnuplot code just now,
>> and noticed that no QApplication object is created anymore in gnuplot.
>> According to the docs, waitForConnected() should also work without an
>> event loop, but I think it would still be interesting to see if the high
>> CPU usage goes away if QCoreApplication is replaced by QApplication.
>> Just my 2cents.
>> Thomas
>
> I don't see any difference here, but then I wasn't seeing CPU-churning
> to begin with.  Let's see what the Windows/OSX people report.
>
> 	Ethan

Here looks like a potential bug:

// Called before a plot to connect to the terminal window, if needed
void qt_connectToServer()
{
	if (!qt)
		return;
	ensureOptionsCreated();

	// Determine to which server we should connect
	bool connectToWidget = !qt_option->Widget.isEmpty();
	QString server = connectToWidget ? qt_option->Widget : qt->localServerName;

	if (qt->socket.state() == QLocalSocket::ConnectedState)
	{
		// Check if we are already connected to the correct server
		if (qt->socket.serverName() == server)
			return;

		// Otherwise disconnect
		qt->socket.disconnectFromServer();
		while (qt->socket.state() == QLocalSocket::ConnectedState)
			qt->socket.waitForDisconnected(1000);
	}

	// Start the gnuplot_qt helper program if not already started
	if (!connectToWidget && !qt->gnuplot_qtStarted)
		execGnuplotQt();

	// Connect to the server, or local server if not available.
	qt_connectToServer(server);
}

At the top of this function is:

	QString server = connectToWidget ? qt_option->Widget : qt->localServerName;

Potentially, localServerName could be an empty string or some valid 
server name.  If localServerName happens to be empty, it's probably 
because the server wasn't connected yet or there was an error in 
connecting.  If that is the case then this test will be true:

	// Start the gnuplot_qt helper program if not already started
	if (!connectToWidget && !qt->gnuplot_qtStarted)
		execGnuplotQt();

But, inside execGnuplotQt() is where the name of qt->localServerName is 
set, so if this line executed "server" will no longer be pertinent. 
Perhaps the first time through "server" is empty and after the valid 
qtgnuplot#### is made the code is requesting for a server with empty string.

Perhaps the code should be more along the lines:

	// Start the gnuplot_qt helper program if not already started
	if (!connectToWidget && !qt->gnuplot_qtStarted)
	{
		execGnuplotQt();
		server = qt->localServerName;
	}

	if (!server.isEmpty())
	{
		// Connect to the server, or local server if not available.
		qt_connectToServer(server);
	}

That isn't very well organized either.

Dan