I like evolution, so I may be wrong but here I am not sure I would flag this as a bug.
Just out of curiosity what undesirable side effect the current implementation generate? ?
Some food for thoughts:
Theses are tricky parts as it require tracking all threads together to do proper analysis. again I won't pretend I know all the implications.
I use it in C++/Win32 and it work really fine and fast (async) without any issues that worth a change from my perspective (IMHO).
Because:
1- If I ask a "disconnect" it's to be acted upon immediately I don't want any new data to be processed.
Exiting gracefully and fast a mutithread application can become a tricky process;
I am ready to live with recv receiving remnant of previous telco. It's a philosophical/aesthetic issue I agree,
in an ideal world, we are 100% sure that the last transaction was completed. yes. but ... we are not (at least I am not ) in this ideal world.
Also the "error" system of IB have to be seen as a client messaging service Same for WIN32 where GetLastError() is sometime used as a signal (Example when looking for enumeration of files, this is the only way to get a "no more file" info.)
The OS handle gracefully zombie packet on close socket, so no harm. (surely not the only apps that do that).
Codes are used to that, just look at handleSocketError you see a hard errno = 0; typical of way to reset it without concerns for unhandled error.
2- Related to #1 above, Putting the WaitForSingleObject(m_hReadThread, INFINITE); before eDisconnect may execute a last putMessageToQueue.
While the basic supplied code will do a hard beak, the while() loop with m_buf.size==0 , exit because m_isAlive==false, without executing anything else.
3- Notice that a EReader object is used during connection as a limited life (scoped) heap object to fetch server protocol revision in a synchronous mode, and to call startApi()
(I am not aware of what this proc? does but by the name of it I would avoid missing this call) this looks like a "trick" to overcome an un-anticipated evolution of the protocol,
See EClientSocket::eConnectImpl(....)
Actions happening in the dctor are then to be taken carefully.
I would agree that the inversion of code lines you suggested might not affect the behavior at it's synchronous, (m_hReadThread=0) , that you ought to get the protocol revision and I don't even see how you can abort this single message operation anyway. (aside of a socket shutdown) never see a need.
As buddy point it out:
- it show that playing with the dctor of EReader may have side effect.
In particular there are legacy code without a call to a ->stop(), ?? it's implicit in ~dctor and somewhere does the job.
- Just to mention, the m_signal used for this Connect EReader is shared by this ephemeral EReader, you most probably will use the same for your main EReader.? It need care, there are people who use more than one EReader on same m_signal
