wxSocketClient & Unread problem Topic is solved

[Widgets]

While working on reading data from a Pop3 server I have run into a curious problem. Now that I have identified it, I can think of a number of ways to overcome it. But all are kludges and depend on specific knowledge of the expected data. ( Windows 8.1, wxWidgets 3.0.2, MSVC2010 Express)
So I am looking for a more general solution - and, hopefully an explanation or hint on how to do this 'properly'
My code looks like:

Code: Select all

bool MyFrame::GetNextLineFromServer( wxSocketClient *pSock, wxString &wsLine )
{
  wxString wsT;
  int i, j;
  int  iLen;
  wxCharBuffer buf2( 256 );
  char buf3[256];
  wsLine =  wxEmptyString;

  while ( true )
  {
   if( !pSock->WaitForRead( 2 ) )
      return false; // must be timeout
    pSock->Read(buf2.data(), sizeof(buf2) );   <<<<<<<<<<<<<<<<< hangs here waiting for Read to 
    iLen = pSock->LastReadCount();                                  return with last 3 bytes
    memset( buf3, '\0', iLen + 1 );
    memcpy( buf3, buf2.data(), iLen );
    wsT = wxEmptyString;
    for ( j = 0; j < iLen; j++ )
    {
      wsT.append( buf3[j] );
      if ( buf3[j] == '\n' )
      {
        wsLine += wsT;
        // put back any unused bytes
        // for the next call
        if( j < (iLen - 1))
        {
          pSock->Unread(&(buf3[j+1]), iLen - j - 1 );
        }
        return true;
      }
    }
    if ( wsT.IsEmpty() )
      return false;
    wsLine += wsT;
  }
  return true;
}

The problem is this: during this part of the response from the POP3 server, the data is expected to arrive as a number of lines, terminated by a line with nothing but ".\r\n".
All is well until the timing and data lengths result in the last buffer read to contain enough data for the next-to-last-line plus the 3 char long terminator line, which then ends up being pushed back to the read buffer in the socket.
BUT, at this time, the server has nothing more to send, so the last read for the terminating line hangs until the socket times out , at which point it does return the expected data, but of course, the server has hung up and won't accept any further request.

My expectation was and still is that the next time Read is called, it would return immediately, because the buffer contains the pushed back line - but it does not!
And only in this specific case. The push back code gets call dozens of times and as long as there is more data coming down the pipe from the server, all works as expected. Except for that last call.
Has anyone else run into this issue and/or how can I avoid it or work around it in a reasonably general way?

TIA
Arnold

[DenDev]

First you should know that I've had some issues with the reliability of wxSocket's. But as far as I can see this line:

Code: Select all

pSock->Read(buf2.data(), sizeof(buf2) );

will block until 256 bytes has been put into buf2 or the socket connection is terminated (or timeout is reached). If you want to read one line from a socket you must use something like:

Code: Select all

bool ReadLineFromSocket(wxSocketClient *socket, wxString &line, long timeout)
{
	
	line.Clear();
	wxStopWatch sw;
	
	while ((timeout <= 0) || (sw.Time() < timeout))
	{
	
		if (socket->WaitForRead(0, 100))
		{
		
			char c;
			socket->Read(&c, sizeof(c));
			if (socket->LastCount() != sizeof(c)) return false;
			if (c == '\n') return true; //End of line reached
			if (c != '\r') line += c; //Add char if not \r
			
		}
		
		else
		{
		
			//If not in a separate thread you might call wxYield() or wxSafeYield()
			//to prevent the app from hanging. But doing this can cause several
			//other problems which can be hard to debug, so be carefull!!
			
		}
		
	}
	
	return false; //Line not received as expected
	
}

If you then want to read the entire set of response headers from the POP3 server, you use:

Code: Select all

bool ReadPOP3Response(wxSocketClient *socket, wxArrayString &as, long timeout)
{

	as.Clear();
	
	do
	{
		
		wxString line;
		if (!ReadLineFromSocket(socket, line, timeout)) return false;
		if (line.Len() == 0) return true; //Terminating line received
		as.Add(line); //Add header line
		
	} while (true);
	
}

I use something like this for all my socket and pipe I/O. The above has been written from memory and has not been tested

[Widgets]

Thank you for your comments, but ....

First you should know that I've had some issues with the reliability of wxSocket's. But as far as I can see this line:

Code: Select all

pSock->Read(buf2.data(), sizeof(buf2) );

will block until 256 bytes has been put into buf2 or the socket connection is terminated (or timeout is reached).

Not according to the documentation and my experience. With the pop3 server I am testing with, the Read( buf....) will typically return with 4-6 bytes.

As I am running the socket code in a secondary thread, I tried to stay away from timers, but I have not tested with a stopwatch. I certainly will give it a try.
Another thing I will have to figure out, is just how using wxStopWatch differs from simply setting the socket timeouts with WaitForRead().
Typically. it seems, that pop3 servers have relatively long timeouts, so it will really be a problematic trade-off between the responsiveness of the link and server and the wait-time for this timeout.

Just the same, I see this as a problem (read: bug ) in the wxSocketBase code.
In all other instances, the bytes put back by Unread() are returned in the next call to Read()

[doublemax]

Unless you have too much free time on your hand or doing it for educational purposes, i'd advise against implementing the POP3 protocol yourself.

I'd just use one of the available libraries like libcurl or vmime.

You could also study the source code for wxEmail:
http://sourceforge.net/projects/wxcode/ ... s/wxEMail/

[DenDev]

wxStopWatch and wxTimer have nothing to do with each other. wxStopWatch is used to determine how much time in milliseconds has elapsed since started or created. It is usefull when doing long tasks that has a timeout. If your sockets are used in separate threads it is important that they uses flag wxSOCKET_BLOCK and either wxSOCKET_NOWAIT or wxSOCKET_WAITALL - the latter is prefferable because it only returns on success or error. Flags are adjusted with socket->SetFlags(wxSOCKET_BLOCK|wxSOCKET_WAITALL).

The samples I provided will give you better possibility to abort long running reads (if needed) and ensure gracefully termination of sockets. I do not get the purpose of using an "Unread"? You should always know how much data to expect and according to any common sense you should read the entire POP3 request / response and handle it within the context of the sockets thread. If you need to "put back" something to the sockets buffer you should reconsider the structure of your code

[Widgets]

Unless you have too much free time on your hand or doing it for educational purposes, i'd advise against implementing the POP3 protocol yourself.

Well, it is not because I have too much time on my hands, but because neither libcurl nor wxEmail were satisfactory in my earlier attempts - which we discussed here as well
As for POP3, my current ISP provides my mail via POP3, so for now that is simply a fact I live with until it changes or I have good reason to switch.

I'd just use one of the available libraries like libcurl or vmime.

I never did get the pop3 examples for libcurl to work as expected and to compile vmime for Windows also does not seem a trivial task, though it might be something I might have to consider (I have abandoned my attempt to compile GMime for windows already, and for the time being am using the mimetic library as it came with wxEMail for that part of the job)

You could also study the source code for wxEmail:
http://sourceforge.net/projects/wxcode/ ... s/wxEMail/

The problem is, I have looked at and studied and tried to use the wxEmail code (and have spent considerable time doing so) only to end up abandoning it because
1) the timers used are incompatible with multi-threading - at least I could find no way to make them work with the thread helper example (and perhaps they are not really needed in any case, given the timeouts in wxSockets) and
2) I am not convinced that the way the input from the server is handled/converted piecemeal (without reference to the encoding for the message body possibly specified in the header) is correct.

Still, either way, the problem I have here is strictly a wxSocket issue, as far as I can see.

Any Pop3 relationship is coincidental and I included that only as sort of background. On second thought, I should have omitted it all together so that the focus stays with the 'real' problem.

The data coming in is line oriented, with nothing known about the total length of each line.
Which makes me conclude, that this issue will surface in other circumstances as well.

If someone can point out why these functions are not intended to be used, or how I am using them incorrectly, I will be very grateful and then I will have to reconsider my approach.

As I have mentioned, now that I understand the problem, I can work around it in various ways, including redesign, but I still would very much like to understand why the current approach is wrong

[Widgets]

wxStopWatch and wxTimer have nothing to do with each other. wxStopWatch is used to determine how much time in milliseconds has elapsed since started or created. It is usefull when doing long tasks that has a timeout. If your sockets are used in separate threads it is important that they uses flag wxSOCKET_BLOCK and either wxSOCKET_NOWAIT or wxSOCKET_WAITALL - the latter is prefferable because it only returns on success or error. Flags are adjusted with socket->SetFlags(wxSOCKET_BLOCK|wxSOCKET_WAITALL).

Since I have not used ywxStopWatch I can't really comment on any issues or problems it might create or solve.
I am reasonably sure all my flags etc are as they ought to be and I have run a good number of tests by now.

The samples I provided will give you better possibility to abort long running reads (if needed) and ensure gracefully termination of sockets.

Understood - which is why I am basing my code on the threadhelper example.

I do not get the purpose of using an "Unread"? You should always know how much data to expect and according to any common sense you should read the entire POP3 request / response and handle it within the context of the sockets thread.

The assumption about knowing how much data to expect is one that cannot be made in this instance - period.
The response to the LIST request simply does not provide that information - and that is where this issue arises, and only in that instance.

If you need to "put back" something to the sockets buffer you should reconsider the structure of your code

I certainly am, though all that does not explain why Unread() should not be used

[doublemax]

I don't know too much about wxSocket, my guess would be that calling Unread when the socket already reached an EOF state does not work as you expect. If that is the case and if you think this is a bug, please open a ticket at http://trac.wxwidgets.org/

In general i would put everything socket related in a worker thread and use blocking I/O. I think this would make your code much simpler, you could just read single bytes from the socket until you reach the end of a line or a timeout/error occurs. As the bytes come from a buffer anyway, i wouldn't expect any performance issues with that approach.

[Widgets]

I don't know too much about wxSocket, my guess would be that calling Unread when the socket already reached an EOF state does not work as you expect. If that is the case and if you think this is a bug, please open a ticket at http://trac.wxwidgets.org/

I suppose, it might be worthwhile, if only to get thew view of the developers more familiar with sockets. By now though, and thanks to this discussion, I believe I understand the problem well enough, that what is happening makes some sort of sense.

I believe you hit the nail on the head with "socket already reached an EOF state", and with blocking, the only way the call will return is due to some timeout or other change in state.
One idea to try would be to set the timeout to quite a bit less than the server timeout to see if it would return the data with a timeout, without waiting for the server to shut the pipe. It might then only wait that long for this special case instead of hanging completely. but see below .....

In general i would put everything socket related in a worker thread and use blocking I/O.

At the start of the socket code in my worker thread I have set the socket flag as you recommend

Code: Select all

m_pSock->SetFlags( wxSOCKET_BLOCK );

and all IO is done in that thread - so that is not the problem.

I think this would make your code much simpler, you could just read single bytes from the socket until you reach the end of a line or a timeout/error occurs. As the bytes come from a buffer anyway, i wouldn't expect any performance issues with that approach.

Those 'read's would then likely have to be done with peek() to avoid any blocking, which, as you suggest, is likely the simplest option.
Another is to check for the case of the last line and, instead of pushing it back, handling it as a special case - which likely means the least changes to my existing code

All in all, any way I slice it, it will mean that it will be code specific to this job
Thank you both for the discussion, I'll mark it as solved

[DenDev]

Even though marked as solved, I believe the issue is not really solved. You are always suggesting that you do not know how much data can be read from the socket - this is in fact not true. If you read through all of the RFC's that declares internet protocols (HTTP, FTP, POP3 and so on) you would know that you can always expect one byte until end of transfer (of headers an empty line or CRLF+CRLF) has arrived. After that you must examine the content of the request / response in order to determine the size (or transfer encoding) of the payload (if any). If you look at my samples they do just that; They read the expected one byte until end of request / response. After this you can examine the headers and do whatever you must. With these protocols there is no valid reason to put anything back to any buffers - this is IMO a flawed design. Also I am not quite sure if you fully understand how pipes and sockets work. You can consider the connection between each endpoint to be a connection to each end of a buffer with a predetermined, fixed size. When one end writes more bytes to the buffer than the buffer can hold the write operation will block (or fail) until the other end reads / removes some content in order to make room for more. You cannot write 1GB to a socket in one blow and expect the data to be stored "somewhere in space and time" until the other end of the connection wants to take care of it. So, if one end feeds a large amount of data to a connection and the other end reads some and puts some back and reads a little again you'll eventually run into trouble. You really need to reconsider your approach to your problem

[Widgets]

Even though marked as solved, I believe the issue is not really solved. You are always suggesting that you do not know how much data can be read from the socket - this is in fact not true.

My answer was to your statement

""You should always know how much data to expect ..

and now your argue a different point regarding ""how much data can be read from a socket" - which is quite a different issue.
As I mentioned, my code is in a secondary thread and while you mention those, you do not provide any code for that case, aside from a caution about what not to do.
In effect, for secondary threads, the wait for more data is a busy waiting loop, with 100 msec probes, which, I would guess is about as good a compromise as any.

.... this is IMO a flawed design.

The very fact that I was posing the question shows that the 'design was flawed'
Quite evidently, a 'proper' design will have to be different and your solution does the 'peek' I was referring to in my reply to Doublemax in a specific way, using wxStopWatch in a very clever way to in effect provide a fine-grained timer compatible with secondary threads.

As a result of this discussion, I have reworked my code in a number of ways (and probably will rework it some more) and am just in the process of testing the end results.
So again, thank you both for your comments and suggestions & code.