A Rewrite of KB192570: An MFC Asynchronous Socket Example Done Right

Home
Back To Tips Page

There was a recent newsgroup posting about asynchronous sockets in separate threads. I pointed out many failures in the code, thinking it had been written by the person who posted the question, who had made a huge number of newbie blunders and errors.  A followup message from the original poster (OP) said that the code was based on Knowledge Base article 192570.

So I went to read the article and the code.  The code represents what is possibly one of the worst examples of asynchronous socket usage ever found anywhere.  It gets threading wrong.  It gets the basic code wrong.  It gets the socket handling wrong.  It commits mind-bogglingly stupid blunders and treats them as if they make sense.

It was so bad that when I describe it to thread-savvy colleagues, I describe it for its humor value. We laugh at how unbelievably poor this example is. Gag lines start out as "And would you believe that...?" and everyone just laughs at how pitiful an example this is. On colleague who is working on his Ph.D. in threading methodologies, and who has developed tools to analyze multithreaded systems, commented "The first thing to understand about threading is that everyone gets it wrong all the time.  That's a close enough approximation to the truth to be applicable everywhere."  I'd love to apply his tools, but they only work on Java code. 

The existence of an example this poor is deeply offensive.  Essentially, I rewrote it for "best practice".

You may notice, if you compare my code with the Microsoft original example, that my code is more complex.  However, it has one real advantage over the Microsoft code: it actually works!  It is not clear the Microsoft code ever worked correctly, and it had no provisions for error detection and reporting.

One of the key lessons here is that it is HARD to write robust, reliable, multithreaded code, and you can't toss off a one-afternoon example and expect it is going to be right.  I have spent DAYS making sure this code is correct, and documenting it.  The toy example in KB192570 is just that: it is a toy.  You don't use toy trucks to haul rocks at real construction sites, and you don't use toy examples to build robust and reliable multithreading code.  The first time you put a large rock in the toy truck, it breaks.  The first time you put a real application on the toy code, it breaks.

Article overview

What's wrong with it?

Overview

Other than the fact that nearly everything is wrong with it, what can I say?  The code is among the poorest I have seen.  It violates so many principles of good coding (not to mention good MFC programming, good thread coding, proper socket handling, and common sense) that I finally gave up writing down every change I made, and just sat down and rewrote it. 

It is not even a good instructional example.  It has poor comments, illustrates very poor practice in a huge number of ways, and is not a good model for derivative code.

Overall, the original code had 1800 SLOC, the rewrite has 3616.  This includes 851 lines of comments I added. Of the original code, 549 lines were removed or changed.  Not counting the lines generated by the AppWizard and ClassWizard, the original code had about 700 lines that were application-specific, and considering I changed 549, this meant that approximately 80% of the original lines were either discarded or recoded. 

I did another comparison.  The socket and threading code in the original example is, eliminating comment lines, blank lines, and boilerplate VS6 stuff, approximately 650  lines.     My code has approximately 960 lines of socket and threading code.  Not a big difference in terms of quantity, but my code actually works, and works in all project configurations. Not counting lines the 194 lines that consist of '{' or '}', of the 112 lines that remained unchanged, 52 of them were generated by wizards and 31 were simple statements like 'else', 'break' and 'return', which means that only about 30 lines survived into the current version completely untouched.  An additional 44 were isomorphic to renaming, meaning that of the original 456 lines, only  74/456 or about 16% actually survived into the rewrite.  So when I say that it was mostly wrong, there are some numbers to prove it.

 It took me a day to rewrite it, and another day to study the code very carefully and make it more robust.  Little things like detailed error checking and reporting, handling multiple Send requests with a queue, creating a stress-test situation for those multiple-send operations, doing a slicker GUI interface, and adding some additional TRACE statements occupied most of that second day.  The core technology, including the FSM, handling and reporting network errors, making it Unicode-aware, and making it 64-bit compatible, took about ten hours; it would have taken less time if VS2005 had not been so tedious to deal with, and would have gone even faster if VS2005 had a mean uptime greater than about an hour before it crashed without warning, thus losing some of my changes and requiring I redo them.  It took me four more days to write this Web page.  One entire day was spent creating hyperlinks.

Frankly, it looks like a project written by a summer intern between his or her freshman and sophomore years, who had spent a week studying MFC and had once heard threads described over the telephone.  And had once, perhaps, read a book on socket programming, but had forgotten most of it.  Where was the adult supervision at Microsoft that approved a project this bad for distribution in the MSDN?

Essentially, this code in the MSDN example is a textbook case of some of the worst possible programming practices that can be used to write any code, anywhere, at any time.  It is incidentally bad networking practice, bad MFC practice, and bad threading practice.

Questionable practice

There is no discussion of why threading is required to handle multiple connections.  Generally, there is no need to use separate threads to handle the multiple asynchronous sockets.  Even if the message that comes in from the socket requires considerable computing, the more effective method is to have all the sockets running in the main GUI thread, and have the extensive computing handled by threads from a thread pool.  This would be a better approach to the design.

However, given the premise that there is a desire to run each socket in a separate thread, the KB example is without a doubt one of the worst examples of how to do multithreaded sockets that could be created.  It isn't even a good framework to start with, given that I did a total rewrite of the algorithms.

Plain bad practice

A number of project-only constants were used in stdafx.h. This is exceptionally poor style, since  stdafx.h is  reserved for standard header files. Project-related constants should be in separate header files.

In addition, this represents a very bad practice known as "lumping", tossing random constants, external declarations, etc. into a single header file.  What was in it?  Well, some message numbers, a buffer size, and a port number.  These represent completely unrelated concepts and belong in completely independent header files.  It is a common newbie error to think that more files are bad.

The values lumped in here were

#define WM_THREADSTART WM_USER+200
#define WM_THREADCLOSE WM_USER+201
#define WM_NEWSTRING WM_USER+202
#define WM_TERM_THREAD WM_USER+203

#define MAX_BUFF 4096
#define PORT_NUM 9898

I moved the message declarations to a separate file called messages.h.  I moved the port number declaration to a separate file called port.h.  And I got rid of the MAX_BUFF value because it had no reason to ever exist in the first place.  Furthermore, the messages used by the client and server were similar, but each project declared its own local copies of messages in its own stdafx.h file.  This was just silly.  Instead, I put the messages.h file in the shared include directory.  I put the shared directory in the project settings of both

The idea of lumping is a lazy programmer's attempt to not do the job right.  There is no excuse for lumping unrelated concepts into a single header file.  I've been known to create a single-line header file (well, two lines, counting the #pragma once) with a single external function or const or #define in it.  Every once in a while, I'll create a global variable, but only after careful thought about its need to be global.  This rarely happens.

In fact, the whole idea of lumping has been proven to be a highly-ineffective development methodology.  With numbers to back up this observation.  See Ellen Borison's Ph.D. dissertation. Program Changes and the Cost of Selective Recompilation, Carnegie Mellon University, Department of Computer Science, 1989.. Her conclusion: the more files you have, the faster your development cycle will proceed. 

There were still lots of things wrong just with these declarations, which I will also address later.

What else is wrong with the project?

Just to show how bad this code is, these constants are DUPLICATED in the stdafx.h file of the client project! So what I did was move the header files of the shared constants to a separate include directory and put that include directory in the project search path of each of the two projects.

The messages are named with a WM_ prefix, which should be reserved for Microsoft-defined messages. So I changed them to UWM_ messages (User Window Message). The values were based on WM_USER, which should be reserved for control-specific messages. I changed them to WM_APP-based messages. Frankly, I consider this poor practice and I would have used Registered Window Messages. But I'll let it go. But I certainly DOCUMENTED the messages!

CAsyncSocket methods which could fail were not tested for success.  Perhaps one of the most egregious failures was the Accept statement, which was not tested for success, but if it failed, the only possible outcome was catastrophic failure that would nearly impossible to analyze.  Errors were either not detected, or detected but not reported.  This is unacceptable practice.

The client code had all the problems of the server code, and in addition it used slightly different variants of the code that sent slightly different messages. The code was changed so that the same connects.h/connects.cpp files are used both by the client and the server, and they use the same messages and the same protocol.

Bad threading practice

The mechanism for passing information to the thread involves a variable. It is also clear that the person who wrote this was clueless about any kind of interthread synchronization. Rather than write a correct program, the program was defined by design to be erroneous, reporting only the most recently received string. An example program should be designed to be an exemplar of correct implementation of a correct design, not a poor implementation of a bad design. This program violates that design principle.

The bad practices this represents is

Thread shutdown was handled badly and this has been fixed.  The way it worked was that a notification was sent to all the server threads to shut down, and then the program waited 5 seconds in the delusional belief that this would guarantee some form of correctness.  This is patently absurd.  There is only one correct way to handle this and that is that as each thread shuts down it sends a notification, and when enough notifications have been received, the main thread initiates its shutdown.

The bad practice this represents is a proclivity among beginners to try to write sequential code in an asynchronous environment.  For example, if three things are to be executed, as in A(); B(); C();, and the desire is to make these into threads. the typical blunder is to write it as

CWinThread * Athread = AfxBeginThread(A,, NULL);
::WaitForSingleObject(Athread->m_hThread, INFINITE);
CWinThread * Bthread = AfxBeginThead(B, NULL);
::WaitForSingleOjbect(Bthread->m_hThread, INFINITE);
CWinThread * Cthread = AfxBeginThread(C, NULL);
::WaitForSingleObject(Cthread->m_hThread, INFINITE);

Note that there are a huge number of errors in this paradigm.  For example, CWinThread objects will be automatically deleted (and the handle closed) when the thread terminates.  So the ::WaitForSingleObject exits not because the thread handle became signaled, but because it became illegal.  Doing this sort of protocol correctly requires a lot more code than is shown here, and yet there are people who deliver what they think are "working" products written this way (I've had to fix them).

It throws away any advantage of threads; by blocking the main GUI thread, the GUI is dead.  It can't respond to user input, it can't log messages from the processes, it can't deal with WM_TIMER messages, and so on.  So there is no reason to have used threads at all! 

But the real problem is that it forces a synchrony on what is a fundamentally asynchronous mechanism.  The correct approach is to start the thread doing A, and when it completes it sends a notification to the main thread that indicates that A is done, and the thread then initiates B.  The thread A doesn't have to know that B will follow.  Suppose that a single parameter value has to be passed to each of them, just to show how this can be done.

AfxBeginThread(A, p);
/* static */ UINT whateverclass::A(LPVOID p)
   {
    CSomething * parms = (CSomething *)p;
    ...do stuff...
    parms->target->PostMessage(UWM_A_DONE, (WPARAM)p);
    return 0;
   } // whateverclass::A
LRESULT CTargetClass::OnADone(WPARAM wParam, LPARAM)
   {
    AfxBeginThread(B, (LPVOID)wParam);
    return 0;
   }

and so on.  We will look at the shutdown for multiple concurrent threads later.

In creating the threads, the constructor for the server thread had an InterlockedIncrement call on a variable. The comment erroneously states that it records the number of threads running; it actually records the number of CServerThread objects created. It is not clear what purpose this variable serves, since a properly-done multithreading system doesn't require the use of an interlocked variable to maintain the count of threads.

The InterlockedDecrement doesn't actually record the fact that the thread has stopped; in addition, the destructor is using a SetEvent which is erroneous because the threads should not be managing this task.  And the main GUI thread should not be blocked waiting for such a notification.

In ClientT.cpp, the ExitInstance handler engages in cross-thread SendMessage. This is seriously erroneous because the main thread might actually be blocked!

For reasons that appear to make no sense, the CWinThread pointer returned by the AfxBeginThread was stored in the socket that was stored in the thread.  Code of the form

pThread->m_strServerName = str; // server machine name
pThread->m_socket.m_pThread = pThread; // the thread that m_socket lives

can best be described as sloppy, and it has the potential to be erroneous.  Why is it that the creator of the thread knows that there is a member called m_Socket and that it has a member called pThread?  For that matter, since it makes no sense to have the thread available, inside the socket, and it is not used inside the socket, there seems to be no rationale for doing this at all.  Why would there need to be a reference to the CWinThread object inside the socket?  This is not good OO programming, and it makes no sense.  The thread handle can be obtained, if needed, by ::GetCurrentThread(), and the thread ID, if needed by ::GetCurrentThreadId().  So there is no need for any reference to the CWinThread object.

The sending logic is completely broken.  If a long message is sent, and part of the sending involves a WSAEWOULDBLOCK return, then the sending thread must wait for an OnSend callback to send the next part of the buffer.  But if an AsyncSendBuff call is issued from the dialog thread during this time, it will completely corrupt the data structures of the program.  This is unbelievably sloppy programming.  There is no synchronization between the two threads to ensure that this cannot happen, and there is no queuing mechanism to make sending work correctly!  There is a comment in the code

// We don't queue up data here.
// If you are going to queue up data packet, it would be better to limit the size
// of the queue and remember to clear up the queue whenever the current packet has been sent.

The first is an observation that the code is erroneous.  The second is an observation that the size of the queue should be limited, although I cannot exactly decide why such a limit needs to exist, and in any case there would be better ways to handle it than are suggested by this comment.  And finally, what is not screamingly obvious about the need to remove an element from the queue, and more likely, before it has been sent.  I suspect the reason a queue was not implemented here is that the author was completely clueless as to how to do it.  And, as you will read in my implementation, it isn't an easy thing to deal with.  But at the very least there should have been a test to see if the previous packet was still in transmission, and to return FALSE if it was; not just proceed on and do bad things.  For example, the test

if (m_nSendDataLen != 0 || nBufLen > MAX_BUFF)
    {
     TCHAR szError[256];
     wsprintf(szError, _TEXT("CConnectSoc::AsyncSendBuff() can't accept more data\n"));
     AfxMessageBox (szError);
     return;
    }

contains so many design errors it is mind-boggling.  This function is called in the context of the main GUI thread.  It is accessing two variables, m_nSendDataLen and nBufLen, which are being modified concurrently in the socket thread, with no form of synchronization.  Then in allocates a fixed-size buffer, uses the dangerous and obsolete wsprintf to format the message, issues a blocking AfxMessageBox call, and simply returns without any indication that there has been a failure! 

Note that it confuses two independent cases: a case where there is an active data transmission in progress, and the case where an incorrect buffer size was given.  It then issues a misleading message.  Note that some end user of the product that contains this library cannot possibly understand what this would mean, or what could be done about it, or the implications.  This might spell total catastrophe, but the programmer who calls this function could never find out that it had failed, or why!

We do not need bad examples of poorly defined interfaces for people to model.

Correct code would set a lock, check the variables, not do any formatting at all, not issue an AfxMessageBox call, and simply return FALSE. The policy of what do do in this case belongs to the caller, and implementing an clearly absurd policy of popping up a Message Box in English is not just poor design, but abysmal design.

Perhaps one of the single worst commercial products I used, a database library, had the misfeature that if it felt you had made an error, it wou/d call ::MessageBox with the desktop as the parent.  This meant that the program would seem to hang, but the message box was underneath the app, and since it had the desktop as a parent, it didn't disable the GUI as a modal dialog box would.  It issued a meaningless error message (to the end user), and returned no indication of error to the programmer.  If you managed to find the dialog, and clicked OK, it would then exit your program.  It was one of the worst exemplars of decent error reporting and recovery I ever saw,  It came with source code, which was so badly written that I decided we had to abandon that product, because nothing short of a complete rewrite could salvage it. 

Here's a real classic:

    ASSERT(m_nBytesRecv < MAX_BUFF && m_nRecvDataLen <= MAX_BUFF);
    nRead = Receive(
                    m_recvBuff + m_nBytesRecv,
                    m_nRecvDataLen-m_nBytesRecv);

Note that it will issue an ASSERT if the size of the packet is larger than the fixed buffer size, but only in the debug version!  It will happily read too much data into the buffer in release mode.  It is this kind of unbelievably slovenly coding that results in buffer overruns in released products--ASSERT is only for debugging, not correctness.  Even in the debug version, after the ASSERT is issued, it still goes ahead and reads more data into the buffer than there is buffer to read it into!  Code like this is so grotesquely irresponsible that it must be killed off.  How anyone could have been allowed to let an example this bad into the MSDN shows that there is truly no adult supervision of these examples.

Bad MFC practice

Controls are manipulated directly instead of using control variables. This has been fixed.

The bad practice here is using GetDlgItem, SetDlgItemText, or SetDlgItemInt to manipulate controls.  Instead, create control variables and manipulate the controls using those.  I feel strongly that anyone who writes more than one or two GetDlgItem calls per year is not using MFC correctly (there are rare exceptions dealing with arrays of controls, and they don't happen very often).

There are places in which a clumsy call to PostMessage is done

::PostMessage(AfxGetApp()->m_pMainWnd->m_hWnd, UWM_THREADSTART, ...);

when a simple PostMessage method would have been better.

AfxGetApp()->m_pMainWnd->PostMessage(UWM_THREADSTART, ...);

More seriously, because this code presumes the main application window is going to get the messages, it is not reusable in a situation where views might be involved, or any other kind of window.

The code is written in MFC, yet the neophyte who wrote this uses fixed-size buffers and wsprintf to format data, instead of using CString::Format. This code is unsalvageable.

The error here is using obsolete and unsafe API calls that can cause buffer overruns, when there is a simpler and reliable mechanism already in MFC.

There are places where OutputDebugString (unconditional tracing call) is called instead of TRACE. These have been fixed.

As is typical for all projects I work on, every trace of the project.h file (AsyncClient.h, AsyncServer.h) were deleted from every file except the corresponding AsyncClient.cpp and AsyncServer.cpp. Microsoft loves to toss these #includes into every generated class file, and they make absolutely no sense to be in any file except the CWinApp-derived class.  So while this represents bad MFC practice, at least this isn't the fault of the original programmer.

There were several places in which the user-defined message handlers were defined to take parameters of UINT, LONG instead of the correct WPARAM, LPARAM. These were fixed. This code would not compile and run properly in a 64-bit environment.

There were several places in which the thread pointer was put into the thread object, even thought it was not used.  In fact, there were a large number of gratuitous variables that were initialized but never used, but which should never have existed at all!  These were all removed.  There is no reason a socket would ever need access to its CWinThread object; if it does, the design is erroneous.

Most methods that had been declared as public were gratuitously public and did not need to be public.  Admittedly, Microsoft got this wrong and largely still has it wrong; for example in VS.NET, all handler methods are declared as public, even though this makes absolutely no sense.  It is necessary to examine every public object to make sure it needs to be, and is not the result of sloppy programming, whether on the part of the programmer or on the part of Microsoft.

Bad GUI practice

The use of OK and Cancel buttons is poor style for dialog-based apps because they could be closed by a stray <enter> or <esc> key.

To fix this, I modified the OK and Cancel handlers and  removed their bodies.  Then I removed the buttons entirely.  I added code to the OnClose handler directly calls CDialog::OnOK() when it is time to shut down. A new button, Close, has been added and an OnBnClickedClose handler initiates shutdown. I describe this in my essay on dialog-based apps.

This code issues AfxMessageBox calls from secondary threads. This is erroneous and has been fixed.  This could also have been listed as bad threading practice.  There is rarely a need to stop a thread, and if there is an error, it can generate notifications in the main GUI thread.

Controls are created as enabled or disabled.  Control state should be computed at run time under all conditions.  Control state should be computed in exactly one place, and based on a dynamic computation (not a bunch of random pieces of code that set Boolean values).  See my essay on Dialog Box Control Management.  I have implemented that model in these apps.

The client code did insane things like SendMessge(WM_CLOSE) to its parent, instead of doing PostMessage(WM_CLOSE). It would be erroneous to do SendMessage.

The GUI will block during shutdown waiting for the threads to shut down, but give the illusion that it is live.  As a general practice, I try to avoid ever doing a ::WaitForSingleObject in the main GUI thread.  Again, the notion that what you want to do is

void CMyMainWindow::OnClose()
   {
    initiate thread shutdown;
    wait for all threads to shut down;
    CMyParent::OnClose();
   }

is another misguided attempt to impose sequentiality on an asynchronous process.  The correct form is

void CMyMainWindow::OnClose()
   {
    if(any threads running)
       { /* asynchronous shutdown */
        set shutdown-in-progress flag;
        initiate thread shutdown;
        return;
       } /* asynchronous shutdown */
     CMyParent::OnClose();
    }

As the thread shutdown sequence progresses, eventually someone notices that there are no more running threads and the shutdown-in-progress flag is set.  At that point, the code looks like

    if(number_of_running_threads == 0 // by some determination
        && shutdown_in_progress)  
       PostMessage(WM_CLOSE);

This removes all need to wait for a shutdown.  In a dialog-based app, some or all controls can be disabled; in an MDI or SDI app, the ON_UPDATE_COMMAND_UI handlers would use the shutdown-in-progress flag to decide what menu items need to be enabled.

(A classic argument against this goes something like "<whine> that takes a lot of work!" and the answer is "Yes it does.  Get over it.".  You can do the job right, or you can do the job wrong.  Products that do the job wrong do not generate happy users.)

Bad network practice

The port number is 9898. This is in the "reserved port number" section of the assignable port numbers, and therefore could potentially conflict with a Registered Port Number. For example, checking the Registered Port Number list at www.iana.org/assignments/port-numbers, we see that this actually is a conflict with a registered port number. So I changed it to 0xC001, an equally random number but one that at least is safe. It is in the private port region.

In a well-designed program, the port address would be settable from a GUI component and stored in the Registry.  I have therefore added this feature, insofar as the GUI component.  I did not save it to the Registry, but I recommend my Registry Classes library for this purpose.

The sending logic is confused. For example, it does a

Send( (LPCTSTR)m_SendBuffer + m_nBytesSent, m_nSendDataLen - m_nBytesSent);

but there is no rationale of why m_SendBuffer, which is declared as a TCHAR, needs to be cast to an LPCTSTR, particularly because the parameter specification for Send is specified as LPVOID; the result is also erroneous in that the arithmetic will not work in a Unicode build. For these purposes, the buffer should be declared as a BYTE buffer. No cast is required.

Like far too many programmers, the person who wrote this somehow assumes that a Receive will receive enough data to be meaningful. In addition, the original programmer chose to use fixed-size buffers, which is nonsensical. I rewrote the receive logic to use a Finite State Machine model which is easier to understand, code, and maintain.

When network callbacks occur (the virtual methods such as OnSend, OnReceive, OnConnect, the error code is ignored.  This is clearly erroneous.  This led to cases where, for example, a failure to connect resulted in the client thinking it actually had a connection.

Bad C++ practice

Here's an excerpt from one of the header files.  It was apparently intended by the programmer that the connects.h/connects.cpp files be shared, but this was not achieved in practice.  But look at this example:

class CConnectSoc : public CAsyncSocket
{
public:
	CConnectSoc();
	virtual ~CConnectSoc();
public:
#ifdef _ASYNCSERVER
	// Pointers to members in CServerDlg

	CString* m_pLastString;
	CCriticalSection* m_pCriticalSection;

	CWinThread* m_pThread;
#endif

        ...
};

What was this person thinking?  A piece of code like this screams "subclassing".  Conditional compilation like this is an exceptionally poor style to use to customize a class for two purposes.  In C++, you would create a superclass, perhaps an abstract superclass (not easy in MFC for reasons discussed below), and then specialize it to the given need by deriving subclasses!  Code like this is inexcusable.

In addition, it is not at all clear why these pointers are used. 

Of course, the fundamental design error that a single CString is used to pass data back from the network thread to the main thread represents an absolutely insane concept, particularly because there is no effort to truly synchronize access so once data is put into the variable, the variable is inaccessible until the data has been removed. This creates that data-losing situation I described earlier.  If such synchronization were provided, it would create a massive performance bottleneck.  So this variable is erroneous in the design. 

The CCriticalSection shows that the programmer actually believed the MFC synchronization primitives are usable, which they are not, and putting a shared pointer here might have been useful, had the need for synchronization been done right.  But I would have used a CRITICAL_SECTION *, except that I believe that the whole notion of interthread synchronization done this way is erroneous.  Proper synchronization would have used a queue and semaphores, and the synchronization primitives would ideally have been hidden in the abstractions of the queue.  For more information on how semaphores would be used, see my essay on semaphores.

Finally, the CWinThread object is totally useless.  Its role is undefined.  It is not actually used in the code, and there is no reason to have it at all, for any purpose.  There is nothing in a CWinThread that is conceivably useful for a socket thread.

Bad Unicode practice

I've already pointed out where the failure of address arithmetic occurs in computing offsets into the Send buffer.  But the code is riddled with Unicode-related errors.

The original apps contain so many coding errors that if they were compiled as Unicode they would completely and utterly fail. This sort of example is no longer tolerable under modern practice.

The code would not work between machines with different endianness.  The code would not work if one app were Unicode and the other were ANSI. 

For that matter, if data were transmitted in Unicode, it would be necessary to convert each Unicode character into Network Standard Byte Order.  Alternatively, you could start the Unicode stream with a Unicode Byte Order Mark (BOM), which would indicate if the Unicode characters were stored in Little Endian or Big Endian order.  This would require an agreement between the sender and receiver to put the BOM as the first Unicode character.

The code reverses the bytes and sends them back.  This works only if characters equal bytes.  Or, in Unicode, if characters are code points.  But Unicode surrogates mean that this will not work correctly, because swapping the code points will produce erroneous Unicode streams.  This misfeature was removed.

Bad Visual Studio Practice

The server and client are essentially independent projects, yet they were done with the one as a subdirectory of the other.  If projects are independent peer projects they should be independent sibling directories under an umbrella parent project.

Bad Documentation Practice

Here's a list of messages.  What do they do, and what are their parameters?

#define WM_THREADSTART WM_USER+200
#define WM_THREADCLOSE WM_USER+201
#define WM_NEWSTRING WM_USER+202
#define WM_TERM_THREAD WM_USER+203

You can't go to the definitions of the hander functions; these are not documented either.  You have to read the code to figure out what is going on here!  It is a common practice with C programmers to think "I've written the code; it is obvious how to use it just by reading it!"  This is the excuse to not do decent documentation of the code.  I've been using the same documentation style for nearly 30 years.  It is reflexive to use it.

Much Better Practice

There are several interpretations as to what constitutes "best" practice.  Some are seriously debatable.  The one thing we can be absolutely certain of, beyond any shadow of a doubt, is that the original Microsoft example does not even come close to mediocre practice, and in some respects represents worst practice.  Everything I have done here uniformly represents massive improvement.  As to whether or not it is "best", well, that's arguable.

Coding practice

Constants which were project wide but duplicated were placed in a common set of header files and placed in a common include directory called include

Code which was the same in both projects was placed in a common directory called, strangely enough, common.

Expressions in #define constructs were parenthesized.

Names were changed to not conflict with reserved Microsoft constants.

I changed the parameters to messages so that if a thread ID is passed for any reason, it is always passed in LPARAM.

Document messages as if they are methods!

Messages are part of the formal interface.  There is nothing quite so amateurish as a sequence of declarations of user-defined messages like the one I found.  I changed it to treat messages as interfaces, and as such, I documented every message (this is the minimum acceptable standard I will accept).

Believe it or not, I once had a client complain that these comments "made the file too hard to read".  I pointed out that the only important part of the file was the comments, and the #defines were mere implementation details, hardly worthy of notice.

/****************************************************************************
*                               UWM_THREADSTART                              
* Inputs:                                                                    
*       WPARAM: unused                              
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT                                                            
*       Logically void, 0, always                                            
* Effect:                                                                    
*       Notifies the main GUI thread that the child thread has started       
****************************************************************************/
#define UWM_THREADSTART  (WM_APP+200)                                        
 
/****************************************************************************
*                               UWM_THREADCLOSE                              
* Inputs:                                                                    
*       WPARAM: unused                                                       
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT                                                            
*       Logically void, 0, always                                            
* Effect:                                                                    
*       Notifies the main thread that a secondary thread has terminated      
****************************************************************************/

#define UWM_THREADCLOSE  (WM_APP+201)                                        
 
/****************************************************************************
*                                UWM_NETWORK_DATA                               
* Inputs:                                                                    
*       WPARAM: (WPARAM)(CByteArray *) Pointer to the data that was received 
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT                                                            
*       Logically void, 0, always                                            
* Effect:                                                                    
*       Passes data to the main thread to be processed and/or displayed
* Notes:                                                                     
*       It is the responsibility of the recipient to delete the CString      
****************************************************************************/

#define UWM_NETWORK_DATA    (WM_APP+202)                                        
 
/****************************************************************************
*                               UWM_TERM_THREAD                              
* Inputs:                                                                    
*       WPARAM: unused                                                       
*       LPARAM: unused                                                       
* Result: LRESULT                                                            
*       Logically void, 0, always                                            
* Effect:                                                                    
*       Sent via PostThreadMessage to the child thread to shut it down       
****************************************************************************/

#define UWM_TERM_THREAD  (WM_APP+203)                                        

/****************************************************************************
*                            UWM_CONNECTIONCLOSE
* Inputs:
*       WPARAM: unused
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT
*       Logically void, 0, always
* Effect: 
*       Notifies the main thread that the connection has closed
****************************************************************************/

#define UWM_CONNECTIONCLOSE (WM_APP + 204)

/****************************************************************************
*                             UWM_CONNECTIONMADE
* Inputs:
*       WPARAM: unused
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT
*       Logically void, 0, always
* Effect: 
*       Notifies the main thread that a connection has succeeded
****************************************************************************/

#define UWM_CONNECTIONMADE (WM_APP + 205)

/****************************************************************************
*                                  UWM_INFO
* Inputs:
*       WPARAM: (WPARAM)(CString *) Connection information message
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT
*       Logically void, 0, always
* Effect: 
*       Used to post trace info about message receip, connections, etc.
* Notes:
*       This only applies when the socket is
*       set to do this (see CConnectSoc::SetInfoRequest)
*       The recipient is responsible for the deletion of the string
****************************************************************************/

#define UWM_INFO (WM_APP + 206)

/****************************************************************************
*                              UWM_NETWORK_ERROR
* Inputs:
*       WPARAM: (WPARAM)(DWORD) error code
*       LPARAM: (LPARAM)(DWORD) Thread id of thread 
* Result: LRESULT
*       Logically void, 0, always
* Effect: 
*       Notifies the main thread that a network error has occurred
****************************************************************************/

#define UWM_NETWORK_ERROR (WM_APP+207)

/****************************************************************************
*                              UWM_SEND_COMPLETE
* Inputs:
*       WPARAM: unused
*       LPARAM: (LPARAM)(DWORD) Thread id of thread that completed sending
* Result: LRESULT
*       Logically void, 0, always
* Effect: 
*       Notifies the owner that the last Send has completed
****************************************************************************/

#define UWM_SEND_COMPLETE (WM_APP+208)

/****************************************************************************
*                                UWM_SEND_DATA
* Inputs:
*       WPARAM: (WPARAM)(CByteArray*) Reference to a CByteArray to be sent
*       LPARAM: unused
* Result: LRESULT
*       Logically void, 0, always
* Effect: 
*       Enqueues a send request
* Notes:
*       This is sent to the thread via PostThreadMessage
*       The sender must allocate the CByteArray on the heap.  The network
*       layers will delete this object when it has been sent
****************************************************************************/

#define UWM_SEND_DATA (WM_APP + 209)

/****************************************************************************
*                            UWM_START_NEXT_PACKET
* Inputs:
*       WPARAM: unused
*       LPARAM: unused
* Result: void
*       
* Effect: 
*       This is sent via PostThreadMessage from the network thread to itself
*       to initiate the asynchronous dequeue of the next pending packet
****************************************************************************/

#define UWM_START_NEXT_PACKET (WM_APP + 210)

Every function that is in my code, even the functions generated by the ClassWizards, gets a header like one of the above.  There is no excuse to not be at least this thorough in writing code.  Messages are just functions with a different wrapper, and they deserve to be documented as first-class entities.  The nice thing is that my editor can drop one of these headers in with about three keystrokes and the name of the function.  Since it can also generate an empty function body, this means it takes me two keystrokes and the name of the function to add the function and its documentation in a single operation. (It takes one extra keystroke to say "Drop in a header, the body is already here").  And people wonder why I don't use the VS editor!   It is hard to respect a program editor that can't even get indentation right!

Network practice

The port number was moved from the reserved area (where it actually conflicted with an official IANA-registered port number) to the temporary/private port number area.

The port number can be changed at runtime.  This is essential to allow in an product code, unless you have an IANA-registered port number, and even if you do, it is still good practice to allow it to be set, because a target site might have a business-critical app that is erroneously coded to conflict with your registered port number, and you should accommodate this situation.

The Receive call is implemented as a finite-state machine which can read partial values for the length and will read arbitrary length strings.

All callbacks that take an error code parameter actually check the error code.  At the moment, they all terminate the connection, but unlike the previous example, they actually notice that an error occurred, and they report the error.  The previous example simply ignored the errors, which could lead to cascading error conditions, and failure to report serious problems (try connecting to a nonexistent server in the Microsoft example.  Study carefully the highly informative error message that tells you the server was not found.  Oh, you didn't see it?  Hmmm... maybe because it isn't there?)

MFC practice

I believe that objects such as collections should be used to manage sequences, rather than raw allocation of bytes, whenever possible.  I have changed the code to use CByteArray as the means of communication of network buffers.

To retain neutrality, I assume that the data might be binary and therefore contain embedded NUL characters, making representation of a string problematic.  So all data is passed into the messaging level as a CByteArray and all received data is sent back from the messaging layer as CByteArray.

Since these apps are passing string data, at the application level (not at the network level) I convert the text strings to UTF-8 representation for transmission. This allows interchangeability between Unicode and ANSI applications.  Note, however, that the representation displayed in an ANSI app may contain ? characters to indicate UTF-8 encoding that cannot be represented as ANSI 8-bit characters.

Member variables and methods not required to be public should (and are) marked as protected

Member variables are assumed to be non-public.  What is declared public are Set and Get functions to update the member variables.  In many cases, the Get functions are not public, or do not exist at all, because the data is write-once, and is read only from within the class.

The CClientThread and CServerThread have so much code in common that I declared them as subclasses of a more general CSocketThread class derived from CWinThread.  Thus the implementation of CClientThread and CServerThread is fairly trivial, with all work being done in the CSocketThread.

I finally got frustrated at the stupidity of VS that puts the CAbout class in the application class and ripped it out to a single pair of files, About.cpp and About.h.  This is because the two About boxes are essentially identical.  It has never made sense, in the entire history of MFC that the CAbout class was not placed in a file of its own.

GUI practice

I centralize all control enabling in a single repository of knowledge, the updateControls method.  Any change that could result in a change in the enabling/disabling, visible/invisible, or any other state of the controls is computed here, and here alone, based on information it derives on demand from the environment.  Much of this information is related to control contents; some is related to state such as having a network connection active.

To make this work, controls that require non-empty values in other edit controls in order to be enabled are disabled.  This eliminated a lot of gratuitous testing that went on in a lot of the functions, because they simply cannot be called if their preconditions are not met.  This saves popping up annoying MessageBoxes during execution.

Controls that can have no effect after a connection is established should be disabled, such as the IP address, port #, etc.

I renamed the messages to be UWM_ messages, but I left them as WM_APP-based messages (a significant improvement from their former WM_USER base).  In my own code, I would never consider using WM_APP-based messages.  I would use Registered Window Messages, as I describe in my accompanying essay.

While it is often the case that an example is stripped down to the basics, the "basics" do not make good examples.  Therefore, I've added features like a ListBox that logs the various messages, state changes, and notifications, and has the additional property that it does not scroll if the last element was not visible when something is appended.  There are few systems more inconvenient than those that do not scroll to show the latest output, and few systems more offensive than those that insist on scrolling to the end when you've scrolled back to look at something.  The ListBoxes used in this project pass both tests of convenience and non-offensiveness. 

My own "best practice" for logging is to use my Logging ListBox Control.

Unicode practice

In order to demonstrate bidirectional transfer between the client and the server,  the server application adds some punctuation and sends the string back.  This means that it will work correctly in the presence of Unicode surrogates.  The original application simply reversed character order, that is, code point order.  It would not work in the presence of Unicode surrogates.

By default, the standard dialog font does not have a full Unicode character set; it has only the characters U0000..U00FF.  So in the OnInitDialog handlers of the applications, I create a Unicode font and replace the fonts of the controls that will have Unicode characters in them. Since this only applies when Unicode characters are possible, I put this under the _UNICODE conditional compilation option.

#ifdef _UNICODE
    if(CreateUnicodeFont(this, font))
       { /* got a font */
        c_LastString.SetFont(&font);
        c_Record.SetFont(&font);
       } /* got a font */
#endif
BOOL CreateUnicodeFont(CWnd * w, CFont & font)
    {
     CFont * f = w->GetFont();
     ASSERT(f != NULL);
     LOGFONT lf;
     f->GetLogFont(&lf);
     StringCchCopy(lf.lfFaceName, sizeof(lf.lfFaceName)/sizeof(TCHAR), _T("Arial Unicode MS"));
     return font.CreateFontIndirect(&lf);
    } // CreateUnicodeFont

Localization practice

All strings that are normally user-visible are in the STRINGTABLE so they can be localized.  Debugging messages are normally directed to the programmer during development and therefore they usually remain in the programmer's native language, which in my case means English. However, I have been known to remove all English-language strings, including debugging strings, and put everything in the STRINGTABLE.

C/C++ practice

I created two specialized subclasses of an abstract superclass, CConnectSoc.  One of them holds information for the server sockets, CServerSocket, and one holds information for the client socket, CClientSocketThe method which will be called to process the packet after receipt is

 virtual void ProcessReceive(CByteArray & ) PURE; // define in subclasses

Therefore, there can never be an instance of a CConnectSoc object.  Since the expectation is that this class must always be subclassed to specify its actual behavior, this is a Good Thing.

Other classes are not quite so fortunate.  For example, there is a substantial amount of common behavior between the CClientThread and the CServerThread classes.  In fact, the only real difference is that one has a CClientSocket for its connection, and the other has a CServerSocket.  I say "nominally abstract superclass" because it is not possible in classes derived from CObject, such as CWinThread, in a manner that  introduces true abstract layers.  If I were to write

 virtual CConnectSoc & GetSocket() PURE; 

which, from the C++ view, makes perfect sense, it won't compile.  The construction

IMPLEMENT_DYNCREATE(CSocketThread, CWinThread)

will produce the error message

ð IMPLEMENT_DYNCREATE(CSocketThread, CWinThread)
I:\mvp_tips\MFCAsync\AsyncClientServer\Common\SocketThread.cpp(20) : error C2259: 'CSocketThread' : cannot instantiate abstract 
                                                            class due to following members:
i:\mvp_tips\mfcasync\asyncclientserver\include\socketthread.h(4) : see declaration of 'CSocketThread'

so instead, I have to write

virtual CConnectSoc * GetSocket() {ASSERT(FALSE); return NULL; } // implement in subclasses

This was a little unfortunate in that I have to return a CConnectSoc * instead of the more reasonable CConnectSoc &, but there is no way to return an "undefined" reference.  Furthermore, I cannot declare something like CConnectSoc dummy; because the CConnectSoc is an abstract superclass and cannot be instantiated. And in any case, I want this to be reported as an error. The comment is there so that if someone gets this error, they will see that they have called a nominal abstract superclass and therefore they cannot proceed until they finish writing that part of the code.

The CClientThread class contains the code

 virtual CConnectSoc * GetSocket() { return &m_socket; }
 CClientSocket m_socket;

and the CServerThread class contains the code

 virtual CConnectSoc * GetSocket() { return &m_socket; }
 CServerSocket m_socket;

I created these projects using error level W4 and "Treat warnings as errors."  (It was interesting that the original project actually does compile under W4 in VS6, at least in the ANSI build.  Since it had no Unicode build and indeed could not produce a valid Unicode version, I did not test it in a Unicode configuration. I was actually surprised that it compiled cleanly at W4).

I did have to make one change to support W4, however.  In some cases, to facilitate debugging, I would write

DWORD err = ::GetLastError();
ASSERT(FALSE);

this made it easier to see what error had occurred.  Due to defective design of the ASSERT mechanism, it resets the last error code, so when the programmer gets control in the debugger, the critical error information has been lost.

But this would generate a warning, 4189, about an initialized but unused variable.  In the modules where I cared, I simply added

#pragma warning(disable,4189)   // allow DWORD err = ::GetLastError() to compile without warning even if err unused

Tracing

When a TRACE statement is issued, it is very good practice to include the name of the function as well.  There are few things less useful than messages that come out and say "Invalid value".  It doesn't matter whether they are done as a MessageBox (see my essay on FormatMessage) or as a TRACE statement.  Maximize the amount of available information!

But in the case of a client/server system, it is a bit trickier.  It is not enough to know that the OnSend or OnReceive methods were called; we need to know which application we're talking about!  When debugging a client/server pair under VS.NET 2003 or later, you can run both the client and the server from the same project in the same version of Visual Studio (right-click on the project name, Debug Ø Start New Instance and you have another app running, whose TRACE output will come out to your output window (at least until you do a compile-and-go, which due to a deep and serious bug in VS, breaks the ability to see TRACE output!)  So all the TRACE statements were modified to be of the form

TRACE(_T("%s: ...stuff here...\n"), AfxGetApp()->m_pszAppName, ...optional parameters here...);

This ensures that the source of the output is always identifiable.  In the example below, I have highlighted the client and server in different colors to make it a little easier to see.  But note that without the identifications of the process name, it would be hard to tell who was sending and who was receiving.

In the protocol shown below, the client and server are started, the client connects to the server, sends the packet "abc", and disconnects.

(Skip around example)

AsyncClient: CConnectSoc::OnConnect(0)
AsyncClient: CConnectSoc::OnConnect: sent UWM_CONNECTIONMADE(0, 0x678)
AsyncClient: CAsyncServerDlg::OnConnectionMade
AsyncClient: CConnectSoc:OnSend(0)
AsyncServer: CListensoc::OnAccept(0)
AsyncClient: CConnectSoc::DoAsyncSendBuff() bytesent=0
AsyncClient: CConnectSoc::DoAsyncSendBuff(): nothing to send
AsyncServer: CConnectSoc::TraceAttach: sent UWM_INFO(0000000000343B50, 0x1648)
AsyncServer: CConnectSoc:OnSend(0)
AsyncServer: CConnectSoc::DoAsyncSendBuff() bytesent=0
AsyncServer: CConnectSoc::DoAsyncSendBuff(): nothing to send
AsyncClient: CAsyncServerDlg::OnSend
AsyncClient: CConnectSoc::Send: sent UWM_SEND_DATA(00000000021E36F0)
AsyncClient: CConnectSoc::DoSendData(00000000021E36F0),0) [6]
AsyncClient: CConnectSoc::StartNextPacket()
AsyncClient: CConnectSoc::DoAsyncSendBuff() bytesent=0
AsyncClient: ConnectSoc::Send(0000000000343B50 + 0 (=0000000000343B50), 10 - 0 (=10), 0)
AsyncClient: CConnectSoc::Send(0000000000343B50, 10, 0)
AsyncClient: Packet [10]
AsyncClient: 00 00 00 06 5b 31 5d 61 62 63
AsyncClient: .  .  .  .  [  1  ]  a  b  c
AsyncClient : Send()=>10
AsyncClient: CConnectSoc::DoSendComplete: sent UWM_SEND_COMPLETE(0, 0x678)
AsyncClient: CConnectSoc::DoSendComplete: sent UWM_START_NEXT_PACKET
AsyncClient: CConnectSoc::StartNextPacket()
AsyncClient: CConnectSoc::StartNextPacket: Queue empty
AsyncServer: CConnectSoc::OnReceive(0)
AsyncServer: CConnectSoc::Receive(00000000021E2C70, 4, 0)
AsyncServer: CConnectSoc::Receive() got 4 bytes
AsyncServer: Packet [4]
AsyncServer: 00 00 00 06
AsyncServer: .  .  .  .
AsyncServer: CConnectSoc::SetDataState: ByteCount 6 (0x00000006)
AsyncServer: CConnectSoc::OnReceive(0)
AsyncServer: CConnectSoc::Receive(0000000000343B50, 6, 0)
AsyncServer: CConnectSoc::Receive() got 6 bytes
AsyncServer: Packet [6]
AsyncServer: 5b 31 5d 61 62 63
AsyncServer: [  1  ]  a  b  c
AsyncServer: CConnectSoc::SendDebugInfoToOwner: sent UWM_INFO(00000000021E37E0, 0x1648)
AsyncServer: CConnectSoc::SendDataToOwner: sent UWM_NETWORK_DATA(00000000021E3850, 0x1648)
AsyncServer: CConnectSoc::Send: sent UWM_SEND_DATA(00000000021E3950)
AsyncServer: CConnectSoc::DoSendData(00000000021E3950),0) [12]
AsyncServer: CConnectSoc::StartNextPacket()
AsyncServer: CConnectSoc::DoAsyncSendBuff() bytesent=0
AsyncServer: ConnectSoc::Send(00000000021E39E0 + 0 (=00000000021E39E0), 16 - 0 (=16), 0)
AsyncServer: CConnectSoc::Send(00000000021E39E0, 16, 0)
AsyncServer: Packet [16]
AsyncServer: 00 00 00 0c 3e 3e 3e 5b 31 5d 61 62 63 3c 3c 3c
AsyncServer: .  .  .  .  >  >  >  [  1  ]  a  b  c  <  <  <
AsyncServer : Send()=>16
AsyncServer: CConnectSoc::DoSendComplete: sent UWM_SEND_COMPLETE(0, 0x1648)
AsyncServer: CConnectSoc::DoSendComplete: sent UWM_START_NEXT_PACKET
AsyncServer: CConnectSoc::StartNextPacket()
AsyncServer: CConnectSoc::StartNextPacket: Queue empty
AsyncClient: CConnectSoc::OnReceive(0)
AsyncClient: CConnectSoc::Receive(000000000034CB10, 4, 0)
AsyncClient: CConnectSoc::Receive() got 4 bytes
AsyncClient: Packet [4]
AsyncClient: 00 00 00 0c
AsyncClient: .  .  .  .
AsyncClient: CConnectSoc::SetDataState: ByteCount 12 (0x0000000c)
AsyncClient: CConnectSoc::OnReceive(0)
AsyncClient: CConnectSoc::Receive(0000000000344450, 12, 0)
AsyncClient: CConnectSoc::Receive() got 12 bytes
AsyncClient: Packet [12]
AsyncClient: 3e 3e 3e 5b 31 5d 61 62 63 3c 3c 3c
AsyncClient: >  >  >  [  1  ]  a  b  c  <  <  <
AsyncClient: CConnectSoc::SendDataToOwner: sent UWM_NETWORK_DATA(000000000034FC80, 0x678)
AsyncClient: CAsyncServerDlg::OnDisconnect
AsyncClient: CAsyncServerDlg::OnDisconnect: Sent UWM_TERM_THREAD
AsyncClient: CConnectSoc::DoClose
AsyncServer: CConnectSoc::OnClose(0)
AsyncServer: CConnectSoc::DoClose
AsyncServer: CConnectSoc::DoClose: sent UWM_CONNECTIONCLOSE(0, 0x1648)
AsyncClient: CConnectSoc::DoClose: sent UWM_CONNECTIONCLOSE(0, 0x678)
AsyncClient: CConnectSoc::DoClose: sent UWM_INFO(0000000000344450, 0x678)
AsyncClient: CAsyncServerDlg::OnConnectionClose
The thread 'Win64 Thread' (0x678) has exited with code 0 (0x0).
AsyncServer: CConnectSoc::DoClose: sent UWM_INFO(0000000000343B50, 0x1648)
The thread 'Win64 Thread' (0x1648) has exited with code 0 (0x0).
AsyncServer: Thread 0x1648 is removed

go back to start of example

The Rewritten App

On the left, the client allows specifying a dotted quad or a host name as the name for the server to connect to.  I chose "localhost" but I could also have specifed "127.0.0.1".  I left the server port unchanged from the default, 49153 (0xC001), and connected to it.  The connection is asynchronous.  I used the Character Map utility to create a string with some characters in it that are either in the range 128..255 (U0080..U00FF) or some randomly-selected larger Unicode characters.  These applications, as shown in the About box on the right, are the Unicode versions, 32-bit, Debug configuration.  The server used the default port number, and I clicked the Run button to start it before the client.  We see the trace of the socket attachment and then a messge indicating that the transmission was from host 127.0.0.1 via port 1614.  The [10] indicates 10 bytes were received, which is the encoding of the four Unicode characters.  The bytes received, as encoded in UTF-8, are as shown below.  The UTF-8 encoding and decoding is described in a separate essay on my Web site.

Unicode U00A9 U00B6 UFB7B UFB4C
UTF-8 c2 a9 c2 b6 ef ad bb ef ad 8c
Character ©

Note that the dialogs are resizable and can be stretched so that all values are shown.  I did not deal with the issues of putting  horizontal scrollbar on the ListBox; the details of that are given in an accompanying essay.

Project Builds

This project builds under both VS6 and VS 2005.  Under VS2005 it can build as a 64-bit application:

I would have built a VS.NET 2003 project, but Microsoft has made this impossible without a lot of effort, which I was not inclined to expend in this case.  It is not possible to create a project that can build under VS.NET 2003 and VS2005.

Since VS2005 is installed on my 64-bit machine and I wanted to create a 64-bit app, I didn't have a choice.

The conversion from VS6 to VS2005 was very clumsy because the automated aspects of the conversion are full of bugs, so it took a lot of hand-editing of the project (and finally hand-editing the .vcproj file, which was faster) to get it to convert to what I wanted.  Otherwise, the VS2005 project overwrote the VS6 project.

The following configurations are supported in this project:

Target

VS6 VS2005
Configuration Platform Character Set
Debug 32-bit ANSI ü ü
Release 32-bit ANSI ü ü
Debug 32-bit Unicode ü ü
Release 32-bit Unicode ü ü
Debug 64-bit ANSI   ü
Release 64-bit ANSI   ü
Debug 64-bit Unicode   ü
Release 64-bit Unicode   ü

Note that each configuration has both a client and a server component, so the VS2005 "batch build" can build 16 executables if all options are selected.

Build directories

Configuration

Directory
VS6 32-bit ANSI Debug ...\AsyncClientServer\AsyncClient\Debug
...\AsyncClientServer\AsyncServer\Debug
Release

...\AsyncClientServer\AsyncClient\Release
...\AsyncClientServer\AsyncServer\Release

Unicode Debug

...\AsyncClientServer\AsyncClient\AsyncClient___Win32_Unicode_Debug
...\AsyncClientServer\AsyncServer\AsyncClient___Win32_Unicode_Debug

Release

...\AsyncClientServer\AsyncClient\AsyncClient___Win32_Unicode_Release
...\AsyncClientServer\AsyncServer\AsyncClient___Win32_Unicode_Release

VS2005 32-bit ANSI Debug ...\AsyncClientServer\AsyncClient\Win32\Debug
...\AsyncClientServer\AsyncServer\Win32\Debug
Release ...\AsyncClientServer\AsyncClient\Win32\Release
...\AsyncClientServer\AsyncServer\Win32\Release
Unicode Debug ...\AsyncClientServer\AsyncClient\Win32\Unicode Debug
...\AsyncClientServer\AsyncServer\Win32\Unicode Debug
Release ...\AsyncClientServer\AsyncClient\Win32\Unicode Release
...\AsyncClientServer\AsyncServer\Win32\Unicode Release
64-bit ANSI Debug ...\AsyncClientServer\AsyncClient\x64\Debug
...\AsyncClientServer\AsyncServer\x64\Debug
Release ...\AsyncClientServer\AsyncClient\x64\Release
...\AsyncClientServer\AsyncServer\x64\Release
Unicode Debug ...\AsyncClientServer\AsyncClient\x64\Unicode Debug
...\AsyncClientServer\AsyncServer\x64\Unicode Debug
Release ...\AsyncClientServer\AsyncClient\x64\Unicode Release
...\AsyncClientServer\AsyncServer\x64\Unicode Release

Code Details

Navigating the examples

The examples and their explanations are hyperlinked.  Each line that has an explanation has a hyperlink at the end of the line, of the form

     Here there is a line of code // [nn]

Clicking on the hyperlink will go down to the explanation:

[nn] This is an explanation of the code

Clicking in the hyperlink at the left of the explanation will return to the line of code that the explanation is associated with.

Class Summary

          

All socket classes are derived by one or more levels from CAsyncSocket

The CConnectSoc class implements a set of operations that handle the message level, that is, splitting the TCP/IP stream into counted message blocks.  This layer also contains utility functions that will transmit either a debug display or the actual string to the owner window, and defines a virtual method that processes the received text in the context of the receiving thread.

The CListenSoc class implements a listener, and its job is to handle the OnAccept notifications and create threads that use a CServerSocket to communicate to the clients.

The CClientSocket and CServerSocket classes implement the client side and server side of the communication.  Essentially, all they provide is an implementation of the virtual method that handles the thread-local message processing.  For this demonstration project, these functions merely invoke the utility functions that transmit the data to the main GUI thread, but in a real system they would be doing substantial work, and only sending notifications to the main GUI thread to display the results of those computations.

class CConnectSoc

class CConnectSoc : public CAsyncSocket {
    public:
        void SetTarget(CWnd * w) { target = w; }
        CWnd * GetTarget() { return target; }

        void SetInfoRequest(BOOL b) { WantPeerInfo = b; }
        BOOL GetInfoRequest() { return WantPeerInfo; }
        BOOL Send(CByteArray & data, DWORD threadid);

    protected:
        CByteArray m_SendBuff;
        INT_PTR bytesSent;
        BOOL Sending;
        CList<CByteArray *, CByteArray *> Queue;

        CByteArray m_recvBuff;

        typedef enum {L0, L1, L2, L3, DATA, ERRORSTATE} ReceiveState; 
        ReceiveState state; 
        union {
           int ByteCount;
           BYTE data[4];
        } Header;       
        void AbortConnection();
        void SetDataState();
        void NotifyOwnerAboutPacket();
        void SendDataToOwner(CByteArray & data);
        void SendDebugInfoToOwner(CByteArray & data);

        virtual void ProcessReceive(CByteArray & data) PURE; // define in subclasses

        virtual void OnConnect(int nErrorCode);
        virtual void OnClose(int nErrorCode);
        virtual void OnReceive(int nErrorCode);
        virtual void OnSend(int nErrorCode);
        ...other members not discussed in this essay
};

class CClientSocket

class CClientSocket : public CConnectSoc
{
protected:
        virtual void ProcessReceive(CByteArray & data);
        ...other members not discussed in this essay
};

class CServerSocket

class CServerSocket : public CConnectSoc
{
protected:
        virtual void ProcessReceive(CByteArray & data);
        ...other members not discussed in this essay
};

class CListensoc

class CListensoc : public CAsyncSocket
{
 public:
        void SetTarget(CWnd * w) { target = w; }
 protected:
        virtual void OnAccept(int nErrorCode);

        CDWordArray m_threadIDs;
        CWnd * target;
        ...other members not discussed in this essay
};

class CSocketThread

class CSocketThread : public CWinThread {
    protected:
       DECLARE_DYNCREATE(CSocketThread)
    public:
       CSocketThread();
       void Send(CByteArray & data) { CConnectSoc * sock = GetSocket();
                                      ASSERT(sock != NULL);
                                      if(sock != NULL)
                                         sock->Send(data, m_nThreadID);
                                    }
       void SetSocket(SOCKET h) { socket = h; }
       void SetTarget(CWnd * w) { target = w; }
    protected:
       virtual CConnectSoc * GetSocket() {ASSERT(FALSE); return NULL; } // implement in subclasses
                       // for example in CServerThread::GetSocket or CClientThread::GetSocket

       // Used to pass the socket handle from the main thread to this thread.
       SOCKET socket;
       // Target of messages which are posted
       CWnd * target;
       virtual BOOL InitInstance();
       virtual int ExitInstance();
    protected:
       virtual ~CSocketThread();
       afx_msg void OnTermThread(WPARAM, LPARAM);
       afx_msg void OnSendData(WPARAM, LPARAM);
       afx_msg void OnStartNextPacket(WPARAM, LPARAM);
       DECLARE_MESSAGE_MAP()
};

class CServerThread

class CServerThread : public CSocketThread
{
protected:
        CServerSocket m_socket;
        virtual CConnectSoc * GetSocket() { return &m_socket; }

        virtual BOOL InitInstance();
        virtual int ExitInstance();

        ...other members not discussed in this essay
};

class CClientThread

class CClientThread : public CSocketThread
{
public:
        void SetServerName(const CString & name) { m_ServerName = name; }
        void SetPort(UINT p) { m_port = p; }
protected:
        virtual BOOL InitInstance();
        virtual int ExitInstance();
protected:
        CClientSocket m_socket;
        virtual CConnectSoc * GetSocket() { return &m_socket; }

        CString m_ServerName;

        UINT m_port;
        
        ...other members not discussed in this essay
};

class CAsyncServerDlg

class CAsyncServerDlg : public CDialog {
protected:
        CDWordArray m_threadIDs;
        BOOL m_MainWndIsClosing; 
        BOOL CleanupThreads();
        afx_msg void OnClose();
        ...other members not discussed in this essay
};

class CAsyncClientDlg

class CAsyncClientDlg : public CDialog
{
// Construction
    public:
       CAsyncClientDlg(CWnd* pParent = NULL);  // standard constructor

    protected:// Dialog Data
       CClientThread* m_pClientThread;
       CString m_ServerName;
       BOOL m_Connected;
       BOOL m_Connecting;
       BOOL m_MainWndIsClosing;
       BOOL m_Sending;
       
protected:
        CFont font;
        BOOL CleanupThread();
        
        void updateControls();

        LRESULT OnInfo(WPARAM, LPARAM);
        LRESULT OnConnectionClose(WPARAM, LPARAM);   // a connection has been closed
        LRESULT OnNetworkData(WPARAM, LPARAM);       // received new message
        LRESULT OnConnectionMade(WPARAM, LPARAM);    // pending connection has been established
        LRESULT OnNetworkError(WPARAM, LPARAM);      // a network error occurred
        ...other members not discussed in this essay
};

Threading (part 1) (part 2)

Since the key defect in the original code was its completely erroneous handling of threading, I'm going to discuss in detail what I did here.

The thread is going to require several key pieces of information.  It will need the IP address of the target (or its DNS name), the desired remote port to connect to, and the window to which notifications should be posted.  Rather than set all kinds of variables including members-of-members, a triple of Set methods of the thread are provided

void CClientThread::SetTarget(CWnd *);
void CClientThread::SetServerName(const CString &);
void CClientThread::SetPort(UINT);

Unlike the previous example, there is no need to set thread pointers or other incidentals, since they serve no useful purpose.  In the spirit of not binding the port number at compile time, the port number is now a user-definable option at run time.

The listener creates a thread

The thread creation code is now as shown below.  This code is  called when the CAsyncSocket class gets a notification that an Accept will not block.

void CListensoc::OnAccept(int nErrorCode) 
   {
    TRACE(_T("%s: CListensoc::OnAccept(%d)\n"), AfxGetApp()->m_pszAppName, nErrorCode);                // [1]
    if(nErrorCode != ERROR_SUCCESS)                                     // [2]
       { /* had an error */
        ASSERT(FALSE);                                                  // [3]
        ASSERT(target != NULL);                                         // [4]
        if(target != NULL)                                              // [5]
           target->PostMessage(UWM_NETWORK_ERROR, (WPARAM)nErrorCode, (LPARAM)::GetCurrentThreadId());  // [6]
        return;
       } /* had an error */
    // New connection is being established

    CAsyncSocket soc;

    // Accept the connection using a temp CAsyncSocket object.
    if(!Accept(soc))                                                    // [7]
       { /* accept failed */
        DWORD err = ::GetLastError();                                   // [8]
        ASSERT(FALSE);                                                  // [9]
        ASSERT(target != NULL);
        if(target != NULL)
           target->PostMessage(UWM_NETWORK_ERROR, (WPARAM)err, (LPARAM)::GetCurrentThreadId());
        return;
       } /* accept failed */

    // Create a thread to handle the connection. The thread is created suspended so that we can
    // set variables in CServerThread before it starts executing.
    CServerThread * pThread = (CServerThread *)AfxBeginThread(RUNTIME_CLASS(CServerThread),
                                                            THREAD_PRIORITY_NORMAL,
                                                            0,
                                                            CREATE_SUSPENDED);  // [10]
    if (pThread == NULL)
        {
         DWORD err = ::GetLastError();                                  // [11]
         ASSERT(FALSE);  // help in debugging                           // [12]
         soc.Close();                                                   // [13]
         TRACE(_T("%s: CListenSoc::OnAccept: Could not create thread: %s\n"), 
                     AfxGetApp()->m_pszAppName, ErrorString(err));      // [14]
         ASSERT(target != NULL);
         if(target != NULL)
            target->PostMessage(UWM_NETWORK_ERROR, (WPARAM)err, (LPARAM)::GetCurrentThreadId());  // [15]
         return;
        }

    // Pass the socket to the thread by passing the socket handle. You cannot pass
    // a CSocket object across threads.
    pThread->SetSocket(soc.Detach());                                   // [16]
    pThread->SetTarget(target);                                         // [17]

    // Now start the thread.
    pThread->ResumeThread();                                            // [18]
        
    CAsyncSocket::OnAccept(nErrorCode);
   }
[1] Having TRACE statements is a serious advantage in debugging.
[2] When a callback method of a socket is called, the nErrorCode value must be checked to see if the operation succeeded
[3] ASSERT(FALSE) is always handy for situations which are expected to be rare, and for which entering the debugger would be an aid to discovering what had gone wrong.
[4] It is a requirement that the SetTarget method be called before doing anything that could cause the OnAccept to occur (that is, before the Listen method is called).  To fail to do so is erroneous, and this ASSERT will catch the error.
[5] Whenever an ASSERT is made, execution can continue, and it is better to fail silently than by taking an access fault.  So this test makes sure that even in the case where the target has not been set, the program will not fail catastrophically.
[6] This posts a notification to the owner of the listening socket that an error has occurs.
[7] The Accept call will create a new local socket to handle the communication.  If it fails, we have to react to this failure.  Otherwise, attempts to create a thread for a non-existent socket will result in some later form of catastrophic failure.
[8] The very first executable line following an API that fails must be the line shown here, that captures the last error.  Doing anything else opens the possibility that the last error code might be altered.  For example, an ASSERT sets the last error code to 0.
[9] Since an Accept is not expected to fail, this is added so that during development there will be a chance for the programmer to intervene at the point where the error occurs.
[10] This is a fairly standard AfxBeginThread call.  However, since the thread cannot be allowed to execute until its parameters have been set, we have a problem.  UI threads do not "take parameters", so the way this is done is to create a suspended thread, set member variables to hold the information, and then allow the thread to run.
[11] This obeys the protocol that the very first executable line after a failing API must capture the error.
[12] Because the thread creation is not expected to fail, this ASSERT allows the programmer to intervene and analyze the cause for failure.
[13] Strictly speaking, this is not required, because the ~CAsyncSocket destructor will close the socket, but it is nonetheless good coding practice to make sure resources are cleaned up.
CAsyncSocket::~CAsyncSocket()
{
	if (m_hSocket != INVALID_SOCKET)
		Close();
}
[14] In this case, a TRACE statement should provide maximum information.  It is inadequate just to issue a message that something failed.  The ErrorString function will be described later.
[15] Notify the owner of the error so the owner can take an action.  In the case of the owner of this socket will log the message in a display.
[16] A CAsyncSocket-derived object cannot be passed across a thread boundary.  There is a map which translates SOCKET values to MFC objects, called a "handle map", which is local to each thread.  A given handle can exist in at most one thread's handle map.  The Detach method removes the SOCKET from the handle map of the current thread.  The SetSocket method stores this SOCKET in a member variable of the target thread.  When that thread starts up, this SOCKET will be bound to the handle map in the running thread using Attach.
[17] The running thread will need to know what window to post notifications to, and the SetTarget method in the derived class stores this CWnd * object so these notifications can be sent.
[18] The thread is now permitted to execute.

CSocketThread::InitInstance

BOOL CSocketThread::InitInstance()
   {
    CConnectSoc * sock = GetSocket();                              // [1]
    ASSERT(sock != NULL);                                          // [2]
    if(sock == NULL)
       { /* failed */
        return FALSE;                                              // [3]
       } /* failed */

    // Post a message to the main thread so that it can update the number of
    // open connections
    ASSERT(target != NULL);                                        // [4]
    if(target == NULL)                                             
      return FALSE;                                                // [5]

    target->PostMessage(UWM_THREADSTART, 0, (LPARAM)m_nThreadID);  // [6]

    m_socket.SetTarget(target);                                    // [7]
    m_socket.SetInfoRequest(TRUE);                                 // [8]

    // Attach the socket handle to a CAsyncSocket object.
    // This makes sure that the socket notifications are sent to this thread.
   if(socket != NULL)
      { /* attach socket */
       m_socket.Attach(m_hSocket);                                 // [9]
       m_socket.TraceAttach();                                     // [10]
      } /* attach socket */
    return TRUE;                                                   // [11]
   }
[1] Call the virtual GetSocket member.  This must be defined in the derived thread classes to return the correct instance of a CConnectSoc.  If this is not implemented in the subclass, the CSocketThread::GetSocket member returns NULL.
[2] We test that this is non-NULL.  If an ASSERT tests a condition, and executing after the ASSERT would cause catastrophic failure, the condition that would cause the failure must be tested.
[3] It is erroneous to have this thread running if it doesn't have a known owner.  Return FALSE to terminate the thread.
[4] It is an error if the CSocketThread::SetTarget has not set a valid target window.
[5] If there is no target, return FALSE to terminate the thread.
[6] Notifies the owner of the thread that the thread is now running.
[7] The socket will also need to notify the owner of the thread, so it is also given a SetTarget method (there is no need to create new names for the same concept, a practice far too many programmers believe in)
[8] There is an optional level of detailed tracing that my socket class can perform, and this call enables that additional level of detailed tracing.
[9] This takes the raw SOCKET handle that was set in this thread and binds it to the thread's local handle map.  Now the callback methods will be able to find the CAsyncSocket object that is associated with this handle.
[10] This provides some tracing information that the socket has been attached.
[11] The InitInstance must return TRUE or the thread will be terminated.

CSocketThread::ExitInstance

int CSocketThread::ExitInstance()
   {
    ASSERT(target != NULL);
    if(target != NULL)
       target->PostMessage(UWM_THREADCLOSE, 0, (LPARAM)m_nThreadID); // [1]
    return CSocketThread::ExitInstance();
[1] This notifies the owner of the thread that the thread has terminated.

CSocketThread::Message Map

BEGIN_MESSAGE_MAP(CSocketThread, CWinThread)
        //{{AFX_MSG_MAP(CServerThread)
        //}}AFX_MSG_MAP
        ON_THREAD_MESSAGE(UWM_TERM_THREAD, OnTermThread)   // [1]
        ON_THREAD_MESSAGE(UWM_SEND_DATA, OnSendData)       // [2]
        ON_THREAD_MESSAGE(UWM_START_NEXT_PACKET, OnStartNextPacket) // [3]
END_MESSAGE_MAP()
[1] When it is desirable to shut this thread down, the owner of the thread will post a UWM_TERM_THREAD notification. The handler of this message will cause the thread to terminate.  It will post this message to the thread, which has no window, by using PostThreadMessage.
[2] Whenever an initiating thread wishes to start another data packet transmission, it will PostThreadMessage of a UWM_SEND_DATA message to the target, which will have a pointer to the buffer to be transmitted.
[3] .Whenever the network has successfully sent the packet and is ready to dequeue the next packet, it will to a PostThreadMessage a UWM_START_NEXT_PACKET to itself.

CSocketThread::OnTermThread

void CSocketThread::OnTermThread(WPARAM, LPARAM)           // [1]
   {
    CConnectSoc * sock = GetSocket();                      // [2]
    ASSERT(sock != NULL);
    if(sock != NULL)
       sock->DoClose();                                    // [3]
    ::PostQuitMessage(0);                                  // [4]
   }
[1] The return type of a thread message is void.  This limitation is enforced by VS.NET and beyond.  Even though the parameters are not used, they must be supplied.  In VS6, omitting this parameters will cause a catastrophic failure of the program.  In VS.NET and beyond the program will not compile.  Note that the types of the parameters are WPARAM and LPARAM, not UINT or LONG.  In 64-bit windows, the UINT, LONG will not compile and would be erroneous if it did compile.
[2] Call the virtual method of the derived class to get a CConnectSoc pointer to the socket involved.  This could be a CClientSocket or a CServerSocket.
[3] Forces the socket to close.
[4] This causes the thread to terminate.

CSocketThread::OnSendData

This function initiates the asynchronous dequeueing loop.  Ultimately, it will either cause the next element to be removed from the list, or it will clear the Sending flag.

 ON_THREAD_MESSAGE(UWM_SEND_DATA, OnSendData)

void CSocketThread::OnSendData(WPARAM wParam, LPARAM lParam)  // [1]
    {
     CConnectSoc * sock = GetSocket();                        // [2]
     ASSERT(sock != NULL);
     if(sock != NULL)
        sock->DoSendData(wParam, lParam);                     // [3]
    } // CSocketThread::OnSendData
[1] The return type of a thread message is void.  This limitation is enforced by VS.NET and beyond.  Even though the parameters are not used, they must be supplied.  In VS6, omitting this parameters will cause a catastrophic failure of the program.  In VS.NET and beyond the program will not compile.   Although the specification of the message UWM_SEND_DATA does not use the LPARAM, I chose to forward it to the handler method of the socket, so it is named
[2] Call the virtual method of the derived class to get a CConnectSoc pointer to the socket involved.  This could be a CClientSocket or a CServerSocket.
[3] Forces the socket to close.

The Client/Server socket

The client and server socket code is identical.  In the case of a client, the OnAccept method would not be called; in the case of the server, the OnConnect method would not be called, but the code is otherwise identical.  In this project, I put the single module in a common directory and included it in both the client and the server project.

CConnectSoc::DoClose

void CConnectSoc::DoClose()
    {
     TRACE(_T("%s: CConnectSoc::DoClose\n"), AfxGetApp()->m_pszAppName); // [1]
     if(m_hSocket == INVALID_SOCKET)                                  // [2]
        { /* not connected */
         ::PostQuitMessage(0);                                        // [3]
         return;                                                      // [4]
        } /* not connected */     
     ShutDown();                                                      // [5]
     Close();                                                         // [6]

     TRACE(_T("%s: CConnectSoc::DoClose: sent UWM_CONNECTIONCLOSE\n"), 
                                        AfxGetApp()->m_pszAppName);   // [7]
     ASSERT(target != NULL);                                          // [8]
     if(target != NULL)
        target->PostMessage(UWM_CONNECTIONCLOSE, 0, (LPARAM)::GetCurrentThreadId()); // [9]
     if(WantPeerInfo)                                                 // [10]
        { /* show close notice */
         CString closed;
         closed.LoadString(IDS_CLOSED);                               // [11]
         if(target != NULL)
            { /* send it */
             CString * s = new CString;                               // [12]
             s->Format(_T("%s %s"), (LPCTSTR)GetPeerPrefix(), closed);// [13]
             if(!target->PostMessage(UWM_INFO, (WPARAM)s, (LPARAM)::GetCurrentThreadId())) // [14]
                { /* failed to send */
                 ASSERT(FALSE);                                       // [15]
                 delete s;                                            // [16]
                } /* failed to send */
            } /* send it */
        } /* show close notice */
     ::PostQuitMessage(0);                                            // [17]
    } // CConnectSoc::DoClose
[1] Tracing is always a good idea in development.
[2] If we are not connected, this is a redundant call.
[3] This will force the thread to terminate
[4] We return, and the posted WM_QUIT will be deueued and processed
[5] The ShutDown method must be called to guarantee that all pending buffers are sent.  This is standard WinSocket protocol.
[6] After the ShutDown method returns, the socket may be safely closed.
[7] When debugging messaging, it is useful to trace the sending and receiving of messages
[8] Make sure that the target reference is valid
[9] Post a message noting that a connection has been closed. 
[10] If additional tracing has been enabled, generate additional trace information.
[11] Because this message will be viewed by a user, we allow for the possibility of localization by loading the message from the STRINGTABLE.
[12] When an object is passed across thread boundaries, the standard protocol is to allocate a heap object and pass its pointer to the receiving thread.   It is the responsibility of the receiving thread to delete this object.  See my essy on Worker Threads.
[13] The contents of the string are formatted
[14] Attempt to post the message.  Note that PostMessage can fail, and if it does, there will be a storage leak because there is no recipient to delete the object
[15] This is an unexpected condition.  During development, give the programmer a chance to intervene
[16] If the PostMessage failed, we must delete the string
[17] This will terminate the thread

CConnectSoc::OnClose

void CConnectSoc::OnClose(int nErrorCode) 
   {
    TRACE(_T("%s: CConnectSoc::OnClose(%d)\n"), AfxGetApp()->m_pszAppName, nErrorCode);
    if(nErrorCode != ERROR_SUCCESS)
       { /* report error */
        ASSERT(target != NULL);
        if(target != NULL)
           { /* send error */
            TRACE(_T("%s: CConnectSoc::OnClose: sent UWM_NETWORK_ERROR(%d, 0x%x)\n"), 
                              AfxGetApp()->m_pszAppName, nErrorCode, ::GetCurrentThreadId());
            target->PostMessage(UWM_NETWORK_ERROR, (WPARAM)nErrorCode, (LPARAM)::GetCurrentThreadId());
           } /* send error */
       } /* report error */
           
    CAsyncSocket::OnClose(nErrorCode);
    DoClose();
   }

Receiving data

The code in the Microsoft example was so convoluted I could not even understand it.  Code should be simple, straightforward, easy to write, easy to debug, and easy to explain. The model I'm showing here is the model of TCP/IP Receive that I have used successfully for over a decade.  It consists of a finite-state machine (FSM) representation of the states.  In TCP/IP, there can be no assumption about the amount of information received or where boundaries are found.  Therefore, I make as many calls as required to read the length of the packet, then as many calls as required to read the data.

While I could write far more complicated code to do this, I feel no compulsion to do so.  FSMs are simply easier to write, debug, and maintain than most ad hoc algorithms that embed the state in nested-if clauses with weird and possibly interacting variables.

A packet stream is a BYTE stream.  It is not a char stream, and it is not a TCHAR stream, and it is not a WCHAR stream.  Therefore, at the protocol level, no interpretation should be put on the meaning of these bytes other than that which is imposed by the packet logic.  All other interpretations, such as characters, would be done by higher levels.

The format of the data is

length

data

L0 L1 L2 L3  

The byte order of the length is one of the issues that is critical.  In a "little-endian" machine like the x86 architecture, the first byte (L0) would be the low-order byte of the length, L1 would be the next-higher-byte, L2 the third-highest byte, and the L3 would be the high-order byte.  Thus, a length of 46342 bytes, hex value 0xB506, would be represented as

length

data

06 B5 00 00

...46,342 bytes of data...

The problem with this is that not all architectures are little-endian.  If this data is read into an int as 4 bytes from the nework, and read by a SPARC computer (as one example of a "big-endian" architecture, it would see the value 0x06B50000, and think it had a message 112,525,312 bytes long. 

To avoid this sort of confusion, a standard known as "Network Standard Byte Order" is used.  Network Standard Byte Order is big-endian, and therefore would represent the 46342 bytes as below

length

data

00 00 B5 06

...46,342 bytes of data...

To convert a 32-bit value to Network Standard Byte Order, the htonl (host-to-network-long) function is applied to an int or long or DWORD value.  This function, on a big-endian machine, does nothing, but on a little-endian machine will swap the byte order.  On a little-endian machine, such as the x86, the following holds:

ASSERT(htonl(0x12345678) == 0x78563412);

The htonl produces a 4-byte result that can be put into a message.  Since the sender and receiver both agree that Network Standard Byte Order is used, both code their handlers so that they convert the network packet by using the ntohl (network-to-host-long) function.  On a big-endian machine, this does nothing, and on a little-endian machine it swaps the byte order.

int length = ntohl(data);

To simplify reading this data, a structure is declared

        union {
           int ByteCount;
           BYTE data[4];
        } Header;

The header can be read piecewise into the data member and accessed via the ByteCount member.

The FSM is not a formal FSM, which is driven by input symbols, but one which is driven by input state.  It has the following states

typedef enum {L0, L1, L2, L3, DATA, ERRORSTATE} ReceiveState;
ReceiveState state;

The state transitions are as follows

State Meaning Action Result Next state
L0 Waiting for first byte of length Attempt to read 4 bytes Read 1 byte L1
Read 2 bytes L2
Read 3 bytes L3
Read 4 bytes DATA
Read 0 bytes (no change)
SOCKET_ERROR ERRORSTATE
L1 Waiting for second byte of length Attempt to read 3 bytes Read 1 byte L2
Read 2 bytes L3
Read 3 bytes DATA
Read 0 bytes (no change)
SOCKET_ERROR ERRORSTATE
L2 Waiting for third byte of length Attempt to read 2 bytes Read 1 byte L3
Read 2 bytes DATA
Read 0 bytes (no change)
SOCKET_ERROR ERRORSTATE
L3 Waiting for fourth byte of length Attempt to read 1 byte Read 1 byte DATA
Read 0 bytes (no change)
SOCKET_ERROR ERRORSTATE
DATA Waiting for data Attempt to read length bytes Read n bytes: length-= n; length > 0 (no change)
Read n bytes; length -= n; length == 0 L0
ERRORSTATE An error has occurred do nothing   ERRORSTATE

CConnectSoc::OnReceive

void CConnectSoc::OnReceive(int nErrorCode) 
   {
    TRACE(_T("%s: CConnectSoc::OnReceive(%d)\n"), 
                   AfxGetApp()->m_pszAppName, nErrorCode);                   // [1]

    if(nErrorCode != ERROR_SUCCESS)                                          // [2]
       { /* had error */
        ASSERT(target != NULL);
        if(target != NULL)
           target->PostMessage(UWM_NETWORK_ERROR, (WPARAM)nErrorCode, (LPARAM)::GetCurrentThreadId()); // [3]
        AbortConnection();                                                   // [4]
        return;                                                              // [5]
       } /* had error */

    int nRead = 0;                                                           // [6]

    switch(state)                                                            // [7]
       { /* state */
        case ERRORSTATE: 
           break;
        case L0: 
           nRead = Receive(&Header.data[0], sizeof(int));                    // [8]
           switch(nRead)                                                     // [9]
              { /* L0 next state */             
               case SOCKET_ERROR:                                            // [10]
                  AbortConnection();                                         // [11]
                  break;            
               case 0:                                           
                  break;                                                     // [12]
               case 1:              
                  state = L1;                                                // [13]
                  break;            
               case 2:              
                  state = L2;                                                // [14]
                  break;            
               case 3:              
                  state = L3;                                                // [15]
                  break;            
               case 4:              
                  SetDataState();                                            // [16]
                  break;            
              } /* L0 next state */ 
           break;                   
        case L1: // one byte already read                                    // [17]
           nRead = Receive(&Header.data[1], sizeof(int) - 1);                // [18]
           switch(nRead)                                                     // [19]
              { /* L1 next state */ 
               case SOCKET_ERROR:   
                  AbortConnection();
                  break;            
               case 0:              
                  break;            
               case 1:              
                  state = L2;       
                  break;            
               case 2:              
                  state = L3;       
                  break;            
               case 3:              
                  SetDataState();   
                  break;            
              } /* L1 next state */ 
           break;                   
        case L2:  // two bytes already read                                  // [20]                  
           nRead = Receive(&Header.data[2], sizeof(int) - 2);                // [21]
           switch(nRead)                                                     // [22]
              { /* L2 next state */
               case SOCKET_ERROR:  
                  AbortConnection();
                  break;            
               case 0:              
                  break;            
               case 1:              
                  state = L3;       
                  break;            
               case 2:              
                  SetDataState();   
                  break;            
              } /* L2 next state */ 
           break;                   
        case L3:                                                             // [23]
           nRead = Receive(&Header.data[3], sizeof(int) - 3);                // [24]
           switch(nRead)                                                     // [25]
              { /* L3 next state */ 
               case SOCKET_ERROR:   
                  AbortConnection();
                  break;            
               case 0:              
                  break;            
               case 1:              
                  SetDataState();   
                  break;            
              } /* L3 next state */ 
           break;                   
        case DATA:                                                           // [26]
           nRead = Receive(m_recvBuff.GetData() + bytesRead, Header.ByteCount); // [27]
           if(nRead == SOCKET_ERROR)                                         // [28]  
              { /* unrecoverable error */ 
               AbortConnection();         
               break;                     
              } /* unrecoverable error */ 
           Header.ByteCount -= nRead;                                        // [29]
           if(Header.ByteCount == 0)                                         // [30]
              { /* all read */            
               NotifyOwnerAboutPacket();                                     // [31]
               state = L0;                                                   // [32]
               break;      
              } /* all read */ 
           break;              
       } /* state */           
}
 
[1] Tracing is always a good idea in development.  Note that this TRACE includes the error code.
[2] The error code is checked.  If it is not ERROR_SUCCESS then a real error occurred, and it will be reported.
[3] The error is reported by posting a message to the owner window.
[4] This will shut the connection down.
[5] If there was an error, we have shut down, there is nothing more to do.
[6] Set the local nRead value to 0. 
[7] This is the top-level dispatcher for the finite-state machine
[8] This attempts to read 4 (sizeof(int)) bytes into the byte array of the header.  Note that it might read 1, 2, 3 or all 4 bytes, depending on where the transmission and packetizing boundaries land, something that is unpredictable.
[9] Based on the number of bytes read, we choose which of the states we are going to move to.
[10] If a SOCKET_ERROR occurs, we will terminate the connection.
[11] This will shut the connection down
[12] Reading 0 bytes mean there was a clean shutdown; we will shortly receive an OnClose notification
[13] One byte has been read in the L0 state, so we move to the L1 state.
[14] Two bytes have been read in the L0 state, so we move to the L2 state
[15] Three bytes have been read in the L0 state, so we move to the L3 state
[16] All four bytes of the length have been read in the L0 state, so we proceed directly to the DATA state via SetDataState
[17] The L1 state means that one byte has been read.  It is otherwise like the L0 state.
[18] We try to read all the remaining three bytes into the length, starting at offset 1 because the 1 byte has been read
[19] Based on the number of bytes read, we proceed to the L2, L3 or DATA states, or deal with the SOCKET_ERROR
[20] The L2 state means that two bytes have been read.  It is otherwise like the L0 state.
[21] We try to read all the remaining two bytes into the length, starting at offset 2 because 2 bytes have been read
[22] Based on the number of bytes read, we proceed to the L3 or DATA states, or deal with the SOCKET_ERROR
[23] The L3 state means that three bytes have been read
[24] We try to read the remaining 1 byte into the length, starting at offset 3 because 3 bytes have been read
[25] Based on the number of bytes read, we proceed to the DATA state, or deal with the SOCKET_ERROR
[26] We are in the DATA state, indicating that we are going to be reading data bytes. 
[27] We read into the data buffer.  The GetData method returns a pointer to the contents of the array, and since this is a CByteArray, the result will be a BYTE *, which means that the correct address will be computed using address arithmetic.  The address we read into is the address in the buffer starting at the bytesRead offset.
[28] If a data read fails, we abort the connection
[29] The remaining bytes to read, in Header.ByteCount, are decremented by the number of bytes read.
[30] If there are no remaining bytes, as computed in Header.ByteCount, we have read the entire packet.
[31]  The NotifyOwnerAboutPacket method will deal with the details of sending this packet to the owner.
[32] Since the entire message has been processed, we return to the L0 state waiting for the next packet to arrive.

The length need not be in binary

Some years ago I was writing the server side of an application.  Someone else was writing the client side.  The problem was that the client side was being written in Visual Basic, and the programmer had no idea how to create a binary integer for transmission, or deal with Network Standard Byte Order.  As it turned out, the typical message length was under 100 bytes, and the longest we could envision was 1024.  So instead of using a binary length and Network Standard Byte Order, we had a protocol that had a header that was defined as four digits giving the length, expressed as characters.

 

length

 

data

d3 d2 d1 d0

;

 

so the message "Hello World" would come as the packet shown below (the hex representation of each byte is in the middle row, the character representation in the bottom row)

 

length

 

data

30 30 31 31

3B

48 65 6C 6C 6F 20 57 6F 72 6C 64
'0' '0' '1' '1'

';'

'H' 'e' 'l' 'l' 'o' ' ' 'W' 'o' 'r' 'l' 'd'

We also established the convention that a packet whose length was specified as 0000; was the end-of-communication indicator and the server would close the connection after receiving such a packet.

I used an additional state of the FSM to ensure that the semicolon was scanned as well.  The semicolon is essentially redundant to ensure we are receiving well-formed packets.  The length, expressed as text, was converted to an integer using atoi (not _ttoi because the protocol is specified in 8-bit characters).

CConnectSoc::NotifyOwnerAboutPacket

This invokes a method to process the packet.  The key here is that the ProcessReceive method is a virtual method, and it is defined as

virtual void ProcessReceive(CByteArray & data) { } // override in subclasses

A socket such as the server socket or the client socket will subclass their sockets from CConnectSoc, and the subclass will implement this virtual method.

void CConnectSoc::NotifyOwnerAboutPacket()
    {
     if(Header.ByteCount > 0)                                                // [1]
        { /* send partial packet */
         // This will truncate the buffer if we have a partial receipt
         m_recvBuff.SetSize(m_recvBuff.GetSize() - Header.ByteCount);        // [2]
        } /* send partial packet */

     if(m_recvBuff.GetSize() == 0)                                           // [3]
        return; // nothing to send

     ProcessReceive(m_recvBuff);                                             // [4]

     m_recvBuff.SetSize(0);                                                  // [5]
     Header.ByteCount = 0;                                                   // [6]
    } // CConnectSoc::NotifyOwnerAboutPacket
[1] If there are any bytes in the packet, we might be called because the socket connection is being aborted.  My choice in this is to allow the message to be processed.  Other implementations might make a different choice.
[2] Compute the desired size of the buffer and truncate it to the number of bytes of data which have been received. 
[3] If nothing was received, or an abort is happening on an empty buffer, just return without any additional processing.
[4] The presumption of why we need a thread to do the handling is that it takes considerable time to process the data received.  To allow for code reuse, we request that a receiver of data will subclass the CConnectSoc class and define this virtual method in the subclass, and in the subclass this will handle the actual processing.  For example, in the class CServerSocket, this virtual method is implemented to send the data to the owner thread, and to echo it back to the client.
[5] We are done with the buffer; delete it.  Because the buffer is in a CByteArray, we do this via SetSize(0)
[6] We also indicate that the remaining byte count is 0. 

CConnectSoc::SendDataToOwner

void CConnectSoc::SendDataToOwner(CByteArray & data)
    {
     if(target != NULL)
        { /* can send */
         CByteArray * result = new CByteArray;                                  // [1]
         result->Copy(data);                                                    // [2]  

         if(!target->PostMessage(UWM_NETWORK_DATA, (WPARAM)result, (LPARAM)::GetCurrentThreadId())) // [3]
            { /* failed to send */
             ASSERT(FALSE);                                                     // [4]
             delete result;                                                     // [5]
            } /* failed to send */
         } /* can send */
    } // CConnectSoc::SendDataToOwner
[1] We are about to log some text, and this requires a cross-thread message transmission.  Allocate an object on the heap to hold the information to be transmitted.
[2] The data is copied into the heap-allocated buffer.
[3] Post the message to the owner.  The owner will be responsible for deleting the object transmitted.
[4] The PostMessage failed.  Allow the programmer to intervene to debug.
[5] Because the PostMessage didn't work, it is necessary to delete the object here.

CConnectSoc::SendDebugInfoToOwner 

This function would be called from a derived class to post the data to the owner window.  The information displayed is in the form shown below.  In this case the string "abc" had been sent from the client.

127.0.0.1 [1403] (3) 61 62 63
void CConnectSoc::SendDebugInfoToOwner(CByteArray & data)
    {
     if(WantPeerInfo)                                                           
        { /* send peer info */
         ASSERT(target != NULL);
         if(target != NULL)
            { /* can send */
             CString * info = new CString;                                      // [1]
             info->Format(_T("%s (%d)"), GetPeerPrefix(), (int)data.GetSize()); // [2]
             for(int i = 0; i < (int)data.GetSize(); i++)
                { /* add bytes */
                 CString t;
                 t.Format(_T(" %02x"), data[i]);
                 *info += t;
                } /* add bytes */
             if(!target->PostMessage(UWM_INFO, (WPARAM)info, (LPARAM)::GetCurrentThreadId())) // [3]
                { /* failed */
                 ASSERT(FALSE);                                                 // [4]
                 delete info;                                                   // [5]
                } /* failed */
            } /* can send */
        } /* send peer info */
    } // CConnectSoc::SendDebugInfoToOwner
[1] .We are about to log some text, and this requires a cross-thread message transmission.  Allocate an object on the heap to hold the information to be transmitted.
[2] Format the data to be transmitted as the IP address and port, the length, and the hex bytes
[3] Post the message to the owner thread
[4] The PostMessage failed.  Allow the programmer to intervene to debug
[5] Because the PostMessage didn't work, it is necessary to delete the object here.

CConnectSoc::SetDataState

void CConnectSoc::SetDataState()
    {                                     
     Header.ByteCount = ntohl(Header.ByteCount);     // [1]
     m_recvBuff.SetSize(Header.ByteCount);           // [2]
     bytesRead = 0;                                  // [3]
     state = DATA;                                   // [4]
    } // CConnectSoc::SetDataState        
[1] The ByteCount field, which overlaps with the data array into which the data has been read, is in Network Standard Byte Order.  This line swaps the bytes to x86 little-endian order
[2] The buffer, which is in the class, is allocated.  Rather than do a new and keep a separate variable for the length, I prefer to combine them into an existing class that manages this correctly.
[3] Set the number of bytesRead, which is the offset into buffer where input will be read, is set to 0.
[4] Sets the state variable so the next OnReceive will read data.

CConnectSoc::AbortConnection

void CConnectSoc::AbortConnection()
    {                              
     NotifyOwnerAboutPacket();                          // [1]
     state = ERRORSTATE;                                // [2]
     DoClose();                                         // [3]
    } // CConnectSoc::AbortConnection
[1] This will send any partial packet.  Note: if it is not desirable to send a partial packet, this line can be removed.
[2] This will shut down the state machine.  No more packets should be processed because there has been an error
[3] Close the socket and terminate the thread.

CConnectSoc::OnConnect

This is called when an asynchronous notification comes in from the network that an asynchronous Connect operation has completed.

void CConnectSoc::OnConnect(int nErrorCode) 
   {
    TRACE(_T("%s: CConnectSoc::OnConnect(%d)\n"), AfxGetApp()->m_pszAppName, nErrorCode);

    if(nErrorCode != ERROR_SUCCESS)             // [1]
        { /* failed */
         AbortConnection(nErrorCode);           // [2]
         return;                                // [3]
        } /* failed */

    CAsyncSocket::OnConnect(nErrorCode);        // [4]

    ASSERT(target != NULL);
    if(target != NULL)
       { /* connection made */
        TRACE(_T("%s: CConnectSoc::OnConnect: sent UWM_CONNECTIONMADE(0, 0x%x)\n"), 
                                             AfxGetApp()->m_pszAppName, ::GetCurrentThreadId());
        target->PostMessage(UWM_CONNECTIONMADE, 0, (LPARAM)::GetCurrentThreadId());  // [5]
       } /* connection made */
   }
[1] It is essential to check the error code when a callback function occurs.  Otherwise, there is no guarantee that the operation succeeded
[2] If the error code indicates a failure, call AbortConnection to terminate the socket.
[3] If there was an error, there is nothing further to do.
[4] Call the superclass.
[5] Notify the target that the connection has been made.

The example below shows a trace of a connection that fails. In this case, the server had not been started.  Shown also is a screen snapshot of the logging window from the dialog.  The error code 10061 is represented by the string "Bad Address".

AsyncClient: CAsyncClientDlg::OnConnect()
AsyncClient: CConnectSoc::OnConnect(10061)
AsyncClient: CConnectSoc::AbortConnection: sent UWM_NETWORK_ERROR(10061, 0x22fc)
AsyncClient: CConnectSoc::DoClose
AsyncClient: CConnectSoc::DoClose: sent UWM_CONNECTIONCLOSE(0, 0x22fc)
AsyncClient: CConnectSoc::DoClose: sent UWM_INFO(00327560, 0x22fc)
The thread 0x22FC has exited with code 0 (0x0).
AsyncClient: CAsyncServerDlg::OnConnectionClose

Threading (part 2) (part 1)

This primarily deals with the issues of thread shutdown.

CAsyncServerDlg::OnThreadStart

As each thread starts up successfully, it notifies the owner of the thread by posting a message.

BOOL CSeverSocket::InitInstance()
    {
     ...
     target->PostMessage(UWM_THREADSTART, (LPARAM)m_nThreadID);           
     ...
    }

In the CAsyncServerDlg we have a handler for this message

LRESULT CAsyncServerDlg::OnThreadStart(WPARAM, LPARAM lParam)             // [1]
   {
    m_open++;                                                             // [2]

    DWORD ThreadID = (DWORD)lParam;                                       // [3]
    m_threadIDs.Add(ThreadID);                                            // [4]

    ShowOpenConnections();                                                // [5]
    return 0;                                                             // [6]
   }
[1] Only the LPARAM is used, so only the LPARAM has a name
[2] Increments the count of currently-open connections (for display purposes only; has no other effect)
[3] Note that in Win64, the assignment
      DWORD ThreadID = lParam;

will generate an error about truncating assignment, since a 64-bit value (LPARAM) is being assigned to a 32-bit value (DWORD).  The explicit cast suppresses the error message.

[4] This appends the new ThreadID to the end of the array.  This is the same as std::vector::push_back.
[5] The ShowOpenConnections method merely updates the display, and does nothing else interesting.
[6] Although this is a PostMessage and the return value has no meaning (no one is looking for it), we must still return a value, and 0 is the generic "don't care" value.

Performing a shutdown

If there are any threads running when we hit the Close button, we must shut them down.  A classic approach to this is to send the threads a series of shutdown notifications, then block the main GUI thread until all the threads have shut down.  This is another attempt to impose a sequential model on a fundamentally asynchronous mechanism.  An erroneous solution is to simply Sleep or impose some other delay mechanism, such as a timed WaitFor..., in the hopes that the value chosen for the wait time makes sense.  Solutions of this nature can generally be dismissed as fundamentally erroneous, and the basic rule is that if you use a Sleep call or other timed delay to make a multithreaded system work, you can be sure that the multithreaded system actually doesn't work, and you have taken what I call the "pixie dust" approach to producing an illusion that the code is working.  Don't worry, it isn't.  You just haven't discovered the timing conditions that will cause it to enter yet a different failure mode.  The illusion is enhanced because the more of the Sleep calls you sprinkle around, the more the illusion of working appears to be true.  The more egregious examples do truly silly things like Sleep(1) in the hopes it will only sleep for a millisecond, which is complete nonsense.

In an asynchronous shutdown, the OnClose handler doesn't wait, but initiates a thread shutdown sequence and returns without calling the superclass handler.  This means that no shutdown occurs.  When the last thread has shut down, the superclass method is called.  One way to accomplish this is to simply post another WM_CLOSE message, and since no threads are running, this will initiate the shutdown.

During the shutdown sequence, it is usually a Bad Idea to let any of the user interface be enabled, so an EnableWindow of the top-level window is performed.

When a shutdown sequence is initiated, each thread is notified that it should shut down, and as each thread shuts down, it sends an asynchronous notification to the main GUI thread,  When all threads have shut down, the close sequence is resumed and the app exits.

CAsyncServerDlg::OnClose

void CAsyncServerDlg::OnClose()                                    // [1]
   {
    m_MainWndIsClosing = TRUE;                                     // [2]
    updateControls();                                              // [3]

    if(!CleanupThreads())                                          // [4]
       { /* threads running */
        TRACE(_T("%s: CAsyncServerDlg::OnClose\n"), AfxGetApp()->m_pszAppName);
        return;                                                    // [5]
       } /* threads running */

    CDialog::OnOK();                                               // [6]
   }
[1] This is the handler called for WM_CLOSE
[2] This flag indicates we have initiated a WM_CLOSE sequence
[3] This enables/disables all controls.  Because the m_MainWndIsClosing flag is set, the updateControls method will disable the entire application by disabling its main window
[4] CleanupThreads will return TRUE if no threads are running, and FALSE if it has initiated a thread shutdown sequence.
[5] If we have initiated a thread shutdown sequence, just return without calling CDialog::OnOK, which would cause the app to terminate.
[6] If no threads were running, close the app.

CAsyncServerDlg::CleanupThreads

BOOL CAsyncServerDlg::CleanupThreads() 
   {
    INT_PTR size = m_threadIDs.GetSize();                                             // [1]
    if(size == 0)                      
       return TRUE;                                                                   // [2]    

    for (INT_PTR i = 0; i < size; i++)                                                // [3]
       { /* scan threads */
        if (!::PostThreadMessage(m_threadIDs[i], UWM_TERM_THREAD, 0, 0))              // [4]
           { /* failed */
            TRACE(_T("%s: CAsyncServerDlg::OnClose: deferring close\n"), AfxGetApp()->m_pszAppName);
            m_threadIDs.RemoveAt(i);                                                  // [5]
           } /* failed */
       } /* scan threads */

    if(m_threadIDs.GetSize() == 0)                                                    // [6]
       return TRUE;                                                                   // [7]
    return FALSE;                                                                     // [8]
}
[1] Compute the size of the thread array.  For compatibility with Win64, this is declared as an INT_PTR.
[2] If there are no threads in the thread array, return TRUE to indicate we are not in a thread shutdown situation
[3] Iterate over the array.  Note that the data type is INT_PTR for Win64 compatibility
[4] Notify each thread tht it should shut down by sending a UWM_TERM_THREAD notification
[5] Since the thread is nonexistent, remove it from the array.
[6] It is possible that all the PostThreadMessage calls failed because the thread ID was illegal, meaning the thread has shut down, so if there are no more threads left in the array...
[7] ...return TRUE to indicate there is not a pending thread-shutdown situation.
[8] Because we are in the midst of a thread shutdown situation, we return FALSE

CAsyncServerDlg::OnThreadClose

LRESULT CAsyncServerDlg::OnThreadClose(WPARAM, LPARAM lParam)              // [1]
   {
    m_open--;                                                              // [2]
    m_close++;                                                             // [3]
        
    ShowOpenConnections();                                                 // [4]
    ShowClosedConnections();                                               // [5]
        
    DWORD dwThreadID = (DWORD)lParam;                                      // [6]
        
    INT_PTR size = m_threadIDs.GetSize();                                  // [7]
    for (INT_PTR i = 0; i < size; i++)                                     // [8]
       { /* scan threads */
        if (m_threadIDs[i] ==  dwThreadID)                                 // [9]
           {
            m_threadIDs.RemoveAt(i);                                       // [10]
            TRACE(_T("%s: Thread 0x%02x is removed\n"), AfxGetApp()->m_pszAppName, dwThreadID);
            break;
           }
       } /* scan threads */

    if(m_MainWndIsClosing && m_open == 0)                                  // [11]
       PostMessage(WM_CLOSE);  // try the close again                      // [12]
    return 0;
   }
[1] Only the LPARAM is used, so only the LPARAM has a name.  The LPARAM is the thread ID of the thread which has just shut down.
[2] Decrement the number of active, open connections.
[3] Increment the number of connections which have been closed.
[4] Display the value of the number of open connections.
[5] Display the value of the number of closed connections
[6] Store the thread ID.  To be compatible with Win64, the (DWORD)
[7] Get the size of the array.  For compatibility with Win64, this must be an INT_PTR.
[8] This simply scans the array looking for a matching thread ID.
[9] If the thread ID matches, it is removed.
[10] The elment is removed from the array and the array is compressed to fill in the gap created by its removal.  The size of the array will be one smaller.
[11] If there are no more open connections, and we are in an app-closing situation, we initiate a new WM_CLOSE.  Note that because there are no more running threads, this will immediately exit.
[12] It is important to use PostMessage to initiate the close, not SendMessage.  Otherwise, the app will close, return here, and chaos will follow.

CServerSocket::ProcessReceive

This is the application-specific handler for the CServerSocket class derived from the generic CConnectSoc.  It performs the following operations on the received data

void CServerSocket::ProcessReceive(CByteArray & data)        // [1]
    {
     SendDebugInfoToOwner(data);                             // [2]
     SendDataToOwner(data);                                  // [3]
     
     CString s = ConvertReceivedDataToString(data);          // [4]
     s = _T(">>>") + s + _T("<<<");                          // [5]
     Sleep(5000); // simulate a lengthy blocking operation   // [6]
     CByteArray msg;                                         // [7]
     ConvertStringToSendData(s, msg);                        // [8]
     Send(msg, ::GetCurrentThreadId());                      // [9]
    } // CServerSocket::ProcessReceive
[1] The input parameter is a CByteArray.  Thus, this could be used to handle arbitrary binary data.  If it is known that only text would be received, and the programming is being done in VS.NET or later, this could be changed to a CStringA data type.
[2] The SendDebugInfoToOwner formats a string (which may include 0 bytes) into a printable trace string and sends it to the owner window.
[3] The SendDataToOwner sends the binary data to the owner.  This is done by making a copy of the binary data on the heap and using PostMessage to transmit it.  Therefore, once this has been called, the data buffer can be reset or modified in any way.
[4] The idea here is to echo the message back to the sender.  The goal is to modify it as a string, but the CByteArray is not a string, it is a UTF-8 encoded byte sequence, so it is not clear how to manipulate it as a string.  The ConvertReceivedDataToString will create a CString value.  Note that in an ANSI app, it is possible that there will be data loss; in a Unicode app there will be no data loss.
[5] This is preparinjg the data for its subsequence processing.  In a real app, this might be wrapping an incoming SQL query with some appropriate syntax or something else fairly trivial, getting ready for the real processing the thread will do.
[6] This line represents the "substantial processing" that the thread must do to the data.  Or lines 4..8 represent the substantial processing, but lines 4..5 are just setup and lines 6..9 are just dealing with sending the reply back to the sender.  So this line would probably be replaced by some long, time-consuming, or blocking operation (perhaps a database query) that is handling the received message.
Regular readers may recollect that I am opposed to putting Sleep calls into code, and this is not placed in the code to "make threading work", or to wait for some asynchronous event that should be sending a positive acknowledgement, or any of the other really bad reasons that are usually given; it is part of a simulation of a workload, and as such would not actually exist in a real application that was doing actual useful work.  It is an animation placeholder for a piece of real code.
[7] Because the transmission representation is UTF-8, and the low-level routines are generalized to transmit any data (including data with embedded NUL characters), the string has to be converted to the low-level transmittal representation.  This is the buffer that is used for that conversion.
[8] The ConvertStringToSendData function takes a string as input and converts it to a UTF-8 encoded byte stream in the buffer which is a CByteArray.
[9] The Send method transmits a data packet.  It puts a length at the front and transmits the rest of the packet.

Sending Data

Sending is substantially simpler than receiving. The basic idea is that a Send method will send some number of bytes, and you will have to wait for a call on OnSend in order to send the next sequence.  So there is simply a bytesSent counter to track the number of bytes sent. 

Lock-free queueing

Nominally, the messages are generated by one thread and handled by the network thread.  If we were to implement queueing, we would have to have a queue such that the initiating thread will enqueue requests on the queue, and the network thread will dequeue the request.  This will involve creating a synchronization primitive, such as a CRITICAL_SECTION or a Mutex, that will allow concurrent access to the queue.  Otherwise, it is impossible to guarantee the correctness of the data structures. 

I believe that the best synchronization is no synchronization.  By this I mean that we should avoid creating situations in which synchronization is required for correctness.  So the way I do this is by not accessing the queue by more than one thread.  Since the only thread that can access the queue is the UI thread that handles the network socket, no locking is necessary.

How do I do this?  By having a method that creates a copy of the buffer on the heap and posts a message to the network thread.  When the network thread receives this message, it simply adds the buffer to the queue.  No lock is involved!

(The truth, of course, is that two locks are involved: there is a lock on the storage allocator when the copy is created, and there is a lock deep in the kernel in the PostThreadMessage handler to manage the message queue for the thread, but both of these locks are user for short intervals and they are not visible to the programmer).

Asynchronous Iteration

This is an event-driven system, driven by asynchronous events represented as messages sent to the message pump.  Therefore, it is important to not block the message pump.

Consider the following scenario: while there are any messages in the queue, initiate sending, and continue until there are no messages left.  You might be tempted to think of coding this as

while(!Queue.IsEmpty()
   {
    CByteArray * data = Queue.RemoveHead();
    Send(data);
   }

This won't work for several reasons.  First, you can't initiate a Send until the current packet is completely sent, so this would fail.   And if this loop were executing, no notifications of other events could be seen, including the completion of the send, until this loop exits.

So one way to handle this is to handle the asynchronous notifications.

void CConnectSock::StartNextPacket()
   {
    CByteArray * data = Queue.RemoveHead();
    Send(data);
   }
CConnectSoc::Send(CByteArray * data)
   {
    ...
    int count = m_Socket.Send(...);
    if(count == data->GetSize())
       StartNextPacket();
   }
void CConnectSoc::OnSend(int nErrorCode)
   {
    ...
    if(bytesSent == length)
        StartNextPacket();

If you study this code, you should discover that if there are 20 items in the queue, and each CConnectSoc::Send operation completes, there is a call on StartNextPacket, which calls Send, which sends the next packet, which is completely sent, which calls StartNextPacket, which calls Send, which...

So there is an unbounded recursion, which could result in a stack overflow, and in addition, control never returns to the message pump.  This means that as long as there is something being processed in this queue, nothing more can be added, because the queue is blocked.  This is overall not a healthy situation to be in.

I solved this by doing an asynchronous loop.  When the send is complete, it does a PostThreadMessage to initiate the next dequeue operation and simply returns, which leads back to the message pump.  This means that the start-next-packet request is merely interleaved with other requests coming in and is handled in FIFO order.  There is no unbounded recursion and there is no blocking of the message pump.

The Send Logic

This is a tutorial essay.  Therefore, I did something I haven't really done in about 40 years or so, I actually drew a flowchar!  The interactions are a bit subtle.  However, for those of you who are students, note that I drew this about five minutes before I typed this sentence in.  The entire design was in my head during the coding phase.  This represents a set of fairly natural paradigms that I don't have to write down to keep straight, but I did want to write down for instructional purposes.

A key idea here is that there are two threads of control, the main GUI thread (red) and the socket manager thread (blue).  Each of these has their own message pump.  Note that some of the methods of CClientThread and CConnectSoc are called in the context of the main GUI thread.  The "handoff" is done when the UWM_SEND_DATA message is queued up by the GUI thread to be processed later by the socket manager thread.  The GUI thread receives notifications via UWM_SEND_COMPLETE.

If it mattered what packet was being acknowledged, note that the code can be extended.  UWM_SEND_DATA has an unused LPARAM field and UWM_SEND_COMPLETE has an unused WPARAM field.  You could assign a sequence number, pass it into the Send logic, add it to the UWM_SEND_DATA message; extended the data structure in the queue to hold a structure which contains this number; and send it back on the UWM_SEND_COMPLETE notification.  I leave this extension as an Exercise For The Reader.  You might also consider rearranging things so that the sequence number was sent and received in the same WPARAM or LPARAM field.

(skip to end of send flowchart)

(skip to start of send flowchart)

CClientThread::Send

This is a method of the controlling thread that is called from the owner of the thread, in the context of the owner thread.  It invokes a method of the socket which is executed in the context of the owner thread.

class CClientThread : public CWinThread {
    public: 
        void Send(CByteArray & data) { m_socket.Send(data, m_nThreadID); }
        ...
};

CConnectSoc::Send

Sending starts with a call to Send.  This creates a copy of the input buffer and sends it to the network thread.  To do this, it must have the thread ID of the thread.  This function is called in the context of whatever thread wishes to initiate the send operation.

void CConnectSoc::Send(CByteArray & data, DWORD threadID)                // [1]
    {
     CByteArray * packet = new CByteArray;                               // [2]

     ASSERT(packet != NULL);
     if(packet == NULL)
        return;
     packet->Copy(data);                                                 // [3]
     TRACE(_T("%s: CConnectSoc::Send: sent UWM_SEND_DATA(%p)\n"), AfxGetApp()->m_pszAppName, packet);
     if(!::PostThreadMessage(threadID, UWM_SEND_DATA, (WPARAM)packet,0)) // [4]
        { /* post failed */
         ASSERT(FALSE);
         delete packet;                                                  // [5]
         return;
        } /* post failed */ 
    } // CConnectSoc::Send
[1] This function is passed a CByteArray containing the data to be sent, and the thread ID of the thread that handles the socket.
[2] I allocate a new CByteArray on the heap which will be a copy of the input request.  This pointer will be passed to the socket's thread.
[3] The Copy operation copies the data from the input parameter buffer to the newly-allocated buffer.
[4] A thread message is placed in the thread queue of the socket thread.  The pointer to the buffer is passed in the WPARAM  field.
[5] If, for some reason, the PostThreadMessage should fail, the buffer would be lost, so it must be deleted here.

CClientThread::OnSendData

The sending is invoked by posting a message to the thread.  The message is handled in the thread; so, for example,

ON_THREAD_MESSAGE(UWM_SEND_DATA, OnSendData)                    // [1]

void CClientThread::OnSendData(WPARAM wParam, LPARAM lParam)    // [2]
    {
     m_socket.OnSendData(wParam, lParam);                       // [3]
    } // CClientThread::OnSendData
[1] This handler is in the message map for the CWinThread-derived class
[2] Although nominally I only care about the WPARAM, I added the LPARAM value so I will not have to make any changes if, for some reason in the future, I decide to use th LPARAM for something.
[3] This calls a method in the socket in the context of the thread that handles the socket.

CConnectSoc::OnSendData

This is executed in the context of the thread, so there will be no need to lock the queue.  This is called from the message handler in the parent thread.  Note that here I do not actually use LPARAM although it is passed in from the higher level.

void CConnectSoc::OnSendData(WPARAM wParam, LPARAM)
    {
     CByteArray * data = (CByteArray *)wParam;                // [1]
     ASSERT(data != NULL);
     if(data == NULL)
        return; // can't do it, get out

     TRACE(_T("%s: CConnectSoc::OnSendData(%p),0) [%d]\n"), AfxGetApp()->m_pszAppName, data, (int)data->GetSize()); // [2]

     Queue.AddTail(data);                                     // [3]

     if(!Sending)                                             // [4]
         StartNextPacket();                                   // [5]
    } // CConnectSoc::OnSendData
[1] The WPARAM is cast back to a CByteArray *.  This is a standard mechanism for passing values across interfaces.
[2] The TRACE logs the pointer and size.
[3] The element is added to the queue.  No locking is required because all access to this queue is from this thread.
[4] If we are not in the midst of doing a send...
[5] ...start the next element from the queue.  Note that there can never be a synchronization error with testing the Sending flag because it is only manipulated by this thread.

CConnectSoc::StartNextPacket

void CConnectSoc::StartNextPacket()
    {
     TRACE(_T("%s: CConnectSoc::StartNextPacket()\n"), AfxGetApp()->m_pszAppName);
     if(Queue.IsEmpty())                                       // [1]
        { /* nothing to send */
         TRACE(_T("%s: CConnectSoc::StartNextPacket: Queue empty\n"), AfxGetApp()->m_pszAppName);
         Sending = FALSE;                                      // [2]
         return;
        } /* nothing to send */

     CByteArray * data = Queue.RemoveHead();                   // [3]
     
     m_SendBuff.SetSize(data->GetSize() + sizeof(UINT));       // [4]
     try {                                                     // [5]
          m_SendBuff.SetSize(data->GetSize() + sizeof(UINT));  // [6]
         }
     catch(CMemoryException * e)                               // [7]
        { /* memory error */
#ifdef _DEBUG
         TCHAR msg[MAX_PATH];                                  // [8]
         e->GetErrorMessage(msg, MAX_PATH);                    // [9]
         TRACE(_T("%s: Allocation error: %s\n"), AfxGetApp()->m_pszAppName, msg); // [10]
#endif
         e->Delete();                                          // [11]
         // Lump all storage errors into "out of memory"
         AbortConnection(ERROR_OUTOFMEMORY);                   // [12]
         return;                                               // [13]
        } /* memory error */

     UINT len = (UINT)data->GetSize();                         // [14]
     len = htonl(len);                                         // [15]
     memcpy(m_SendBuff.GetData(), &len, sizeof(int));          // [16]
     memcpy(m_SendBuff.GetData() + sizeof(int), data->GetData(), data->GetSize()); // [17]
     bytesSent = 0;                                            // [18]
     Sending = TRUE;                                           // [19]
     delete data;                                              // [20]
     DoAsyncSendBuff();                                        // [21]
    } // CConnectSoc::StartNextPacket
[1] The queue is tested for an element being present.
[2]  If there are no elements in the queue, the Sending flag is set to FALSE and we are done here.
[3] If there are elements in the queue, the first element in the queue is removed.  (Note that RemoveHead is undefined if it is applied to an empty list).
[4] The transmit buffer is allocated by SetSize.  The required size is the size of the data value plus enough space to handle the initial length value, sizeof(UINT).
[5] We are about to call a method which will throw an exception if the memory cannot be allocated.  We should be prepared to handle this smoothly.  Otherwise, we will get an annoying and inappropriate MessageBox which will be remarkably uninformative to the end user.
[6] Allocate enough bytes in the buffer to hold the packet that will be received.
[7] This catches any CMemoryException that might be thrown by the failure to allocate storage.
[8] It is most unfortunate that Microsoft has not seen fit to have a GetErrorMessage(CString &) version of this method.  It is also unfortunate that they have not seen fit to document the fact that the maximum message size is actually fixed at a specific value.  So we have to allocate a TCHAR array on the stack, and choose some random value to indicate its maximum length.  I chose, rather arbitrarily, MAX_PATH.
[9] CException::GetErrorMessage retrieves a printable, localized string
[10] Strictly speaking, we should have tested the return value to see if a string was actually returned (it will return FALSE if there was no string) but the behavior is well-defined at this point, and the TCHAR array is set to have a string of length 0, so we can ignore this condition.
[11] After an exception is handled, it must be deleted.
[12]

Here is a decision point: what to do if there is insufficient memory to hold the message?  Possible choices are

  • Abort the connection (the choice here)

  • Discard the message without reading it
  • Read as much of the message as can fit and discard the rest

Aborting the connection is simplest.  Everything shuts down, and the owner of the communication receives a notification that the connection has been shut down due to a lack of memory.  The next two options are highly domain-specific as to whether they make sense or not.  If the option is taken to discard the message, a notification mechanism should be in place to inform the owner of the communication that a packet has been discarded.  Then the DoSendComplete must be called to dequeue the next message for transmission.  Similarly, if the message is truncated, there should be a way to notify the owner of the connection thta this has occurred, and there has to be a way to decide how much storage can be allocated.

The basic idea here of aborting the connection is based on the premise that if the length is too large, probably over a gigabyte, it is likely to indicate a failure in the protocol and a bad length, which will essentially be unrecoverable.  We have been passed a bad buffer length.

[13] If the choice is made to abort the connection, all that is required here is to return.
[14] The length of the data value is retrieved.  This is the number of bytes to be transmitted.
[15] It is converted from host representation to Network Standard Byte Order using htonl.
[16] The memcpy operation is used to copy the bytes of the length field to the transmit buffer.
[17] It is followed by a memcpy operation to copy the bytes of the packet. Note that memcpy is safe because the buffer has been allocated to be of a sufficient size to hold the information.
[18] At this point, we have sent no bytes.
[19] Enter the Sending mode.  Any new requests that come in will be queued up.
[20] Delete the data object.  This leads to the question of why I chose to do a copy, rather than reference the buffer.  This was an arbitrary choice.   If I wanted to reuse the buffer, I would have to shift the bytes upward to make room for the length at the front.  This would take the same amount of time as the copy.  I could implement another FSM to do the sending, but most packets sent over the network are small packets (a few tens of K at most), and adding all this complexity would only impact performance, not correctness (unlike the FSM for receiving data, where correctness is the basis of the complexity).  If I were expecting multi-megabyte packets, I would not do the additional copy, but would use the CByteArray pointer directly, and use an FSM to handle the output states.  Right now the FSM is quite simple: if there are bytes remaining to send, send them, otherwise done.
[21] This initiates the CAsyncSocket::Send operation and starts the transmission.  Sending will continue via OnSend callbacks until all the data is sent.

CConnectSoc::OnSend

This is the callback that is invoked if the CAsyncSocket::Send operation is unable to complete because it returned WSAEWOULDBLOCK.

void CConnectSoc::OnSend(int nErrorCode) 
   {
    TRACE(_T("%s: CConnectSoc:OnSend(%d)\n"), AfxGetApp()->m_pszAppName, nErrorCode);  // [1]

    if(nErrorCode != ERROR_SUCCESS)                                                    // [2]
       { /* had error */
        AbortConnection(nErrorCode);                                                   // [3]
        return;                                                                        // [4]
       } /* had error */

    DoAsyncSendBuff();                                                                 // [5]
    CAsyncSocket::OnSend(nErrorCode);
   }
[1] Do a TRACE which records the error code.
[2] It is essential to check the error code on a callback.  If it is not ERROR_SUCCESS, there is a problem.  My decision was that all network errors abort the connection.
[3] This will shut the socket down.
[4] Given the socket is shut down, there is nothing more to do.
[5] If there was no error, calls DoAsyncSendBuff which sends the next segment of the message to the network.  If this is unable to send all of the remainder of the message, OnSend will be called again when it is possible to send the next packet.

CConnectSoc::DoAsyncSendBuff

void CConnectSoc::DoAsyncSendBuff()
   {
    TRACE(_T("%s: CConnectSoc::DoAsyncSendBuff()\n"), AfxGetApp()->m_pszAppName);

    if(bytesSent == m_SendBuff.GetSize())                           // [1]
       { /* all sent */
        DoSendComplete();                                           // [2]
        TRACE(_T("%s: CConnectSoc::DoAsyncSendBuff(): nothing to send\n"), 
                                      AfxGetApp()->m_pszAppName);
        return;                                                     // [3]
       } /* all sent */

    ASSERT(bytesSent >= 0);                                         // [4]
    LPBYTE p = m_SendBuff.GetData() + bytesSent;                    // [5]
    INT_PTR len = (int)(m_SendBuff.GetSize() - bytesSent);          // [6]
    TRACE(_T("%s: ConnectSoc::Send(%p + %d (=%p),  %d - %d (=%d), 0)\n"),
                   AfxGetApp()->m_pszAppName,
                   m_SendBuff.GetData(), bytesSent, p,
                   (int)m_SendBuff.GetSize(),
                   (int)bytesSent,
                   len);

    INT_PTR result = Send(p, (int)len, 0);                          // [7]
    if(result == SOCKET_ERROR)                                      // [8]
       { /* failed */
        DWORD err = ::GetLastError();                               // [9]
        if(err != WSAEWOULDBLOCK)                                   // [10]
           { /* couldn't send */
            AbortConnection(err);                                   // [11]
            return; // done for now, wait for next call             // [12]
           } /* couldn't send */
       } /* failed */
    else
        { /* succeeded */
         TRACE(_T("%s : Send()=>%d\n"), AfxGetApp()->m_pszAppName, result);
         bytesSent += result;                                       // [13]
         if(bytesSent == m_SendBuff.GetSize())                      // [14]
            DoSendComplete();                                       // [15]
        } /* succeeded */
   } // CConnectSoc::DoAsyncSendBuff
[1] This function would be called for one of two reasons: we are initiating the send of a new packet, or we are continuing the send of an existing packet which could not be fully sent because of a WSAEWOULDBLOCK situation.  In the second case, it might be true that  we have a zero-length packet. 
[2] If there are no remaining bytes to send, we call DoSendComplete to initiate the next queued packet transimission (if no packets are queued, this will do nothing).
[3] We are done, and we just return.
[4] A little check to ensure that there are no negative packet sizes given
[5] For an array of a given <TYPE>, the GetData method returns a <TYPE *> pointer to the first element of the array.  Since this is a CByteArray, which is really a CArray<BYTE, BYTE>, the GetData method returns a BYTE * or LPBYTE value.  By adding the bytesSent value, we get the address in the array where the next Send operation will start.
[6] This computes the length of the remaining message to send. 
[7] Send the data out the socket.
[8] If there was a failure, the result will be SOCKET_ERROR.
[9] It is good policy to capture the error code immediately.  The very first line executed (and I mean, before any constructors or anything else!) after a failig API call must be one that calls ::GetLastError and saves its value.
[10] If the error code was not indicative of an asynchronous blocking situation, WSAEWOULDBLOCK, then there is an actual serious error.
[11] Our default action on a network error is to abort the connection and notify the owner of the socket.
[12] Just get out
[13] If the result was not SOCKET_ERROR, it is the count of the number of bytes transmitted.  Add that count to the current running total of bytes transmitted.  Note that the count of bytes actually sent might well be less than the value len that was passed as the parameter!
[14] If the Send succeeded in sending the entire packet, there will be no OnSend callback. 
[15] Therefore, it is necessary to call DoSendComplete to start the next packet.

CConnectSoc::DoSendComplete

void CConnectSoc::DoSendComplete()
    {
     if(!Sending)                                                                   // [1]
        return;
     ASSERT(target != NULL);
     if(target != NULL)
        target->PostMessage(UWM_SEND_COMPLETE, 0, (LPARAM)::GetCurrentThreadId());  // [2]
     ::PostThreadMessage(::GetCurrentThreadId(), UWM_START_NEXT_PACKET, 0, 0);      // [3]
    } // CConnectSoc::DoSendComplete
[1] If we are not in an active sending state, don't do anything, just return
[2] Post a message to the owner of this thread that a packet has been sent.  This could be used by the owner to implement a policy of flow control; when a packet has been sent, a new packet can be computed.  This allows the owner to implement flow control without having to resort to blocking mechanisms like semaphores.
[3] Post a message to this thread to initiate the next packet.  This keeps us from going into an unbounded recursion (by calling StartNextPacket that calls DoSendComplete that calls StartNextPacket...)  Note that since it is being sent to the current thread via PostThreadMessage, and there is no CWinThread object available to us, we can just use the API call GettCurrentThreadId to obtain the thread ID of this thread.

TheProgrammer's Interface

Client Side

A client that wishes to con nect to a server performs several actions.

Create a suspended thread

    m_pClientThread = (CClientThread*)AfxBeginThread(RUNTIME_CLASS(CClientThread), THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
    if (m_pClientThread == NULL)
       ... deal with thread startup failure

Set the parameters to the thread

There are some variables that are assumed to have been initialized for the code below.  These might be local variables in the function, member variables of the class, or parameters passed in. 

UINT port;           // set to the desired port number of the server
CString ServerName;  // the name of the remote server, a DNS name or a dotted quad
    m_pClientThread->SetTarget(this);            // [1]
    m_pClientThread->SetServerName(ServerName);  // [2]
    m_pClientThread->SetPort(port);              // [3]
[1] The SetTarget method must be called before the thread is allowed to run.  This sets the CWnd * to which notifications will be posted.  Since the notifications are intended to come to the current window, this is passed.
[2] The connection will be made to a server defined by a server name and a port number.  The SetServerName method sets the server name.
[3] The SetPort method sets the port number for the server port.

Allow the thread to resume

This will issue a ResumeThread to start the thread.

The m_Connecting variable will be set.  This would be used by your program cause a menu item, toolbar button, or dialog control to be disabled until the connection either completes or fails.  It would be used as part of the pCmdUI->Enable expression in an ON_UPDATE_COMMAND_UI handler, or in your dialog box logic to enable or disable controls during the connection process.  The connection process can take many seconds; for example, there is a 70 second timeout just to resolve a DNS name, and then there are round-trip times and server response times involved.  So 90 seconds might not be an unreasonable upper bound.  You would only want to disable those parts of the GUI that are relevant to initiating a connection, and not just do something like pop up a dialog box and say "Connecting...."  You will get an asynchronous notification of the status of the connection.

    // Now start the thread.
    m_Connecting = TRUE;                         // [1]
    
    m_pClientThread->ResumeThread();             // [2]
    updateControls();                            // [3]
[1] This boolean value is used to enable/disable the Connect button.  It serves no useful purpose as far as the networking interface, only for the GUI.
[2] The ResumeThread allows the suspended thread to start
[3] The updateControls method will handle control state.  This is purely application-specific.

Handle notification of the connection being made

If the connection is successful, this message will be posted by the network layer.  If the connection fails, a thread termination message, UWM_THREADCLOSE, will be received.

ON_MESSAGE(UWM_CONNECTIONMADE, OnConnectionMade)

LRESULT CAsyncClientDlg::OnConnectionMade(WPARAM, LPARAM)
   {
    TRACE(_T("%s: CAsyncServerDlg::OnConnectionMade\n"), AfxGetApp()->m_pszAppName);
    m_Connected = TRUE;                          // [1]
    m_Connecting = FALSE;                        // [2]        
    m_Sending = FALSE;                           // [3]

    updateControls();                            // [4]
    return 0;
   }
[1] The connection has been made.  Record this fact.
[2] The connection has been made.  Therefore, we are no longer actively connecting.
[3] Just to be certain, we clear the m_Sending flag.
[4] The updateControls method will handle control state.  This is purely application-specific.

Send Packets of data to the thread

In this implementation, you are required to fill in a CByteArray with the bytes to be sent.  In the case of string data, I provide two conversion functions that can convert from a string to a UTF-8 byte sequence, or from a UTF-8 byte sequence in a CByteArray to a string (possibly with data loss for ANSI-mode versions).  These conversions functions are described in a separate essay on UTF-8 Conversion.  In the code snippet below, s is the string to be sent.

        CByteArray data;                         // [1]
        ConvertStringToSendData(s, data);        // [2]
        m_pClientThread->Send(data);             // [3]
        m_Sending = TRUE;                        // [4]
        updateControls();                        // [5]
[1] I chose the CByteArray as my choice of representation. 
[2] The ConvertStringToSendData method takes a const CString & and a CByteArray & and transforms the CString to a UTF-8 representation in bytes.
[3] The Send method will make a copy of the data value, so upon return, the array can be reused, deallocated, etc.
[4] The m_Sending flag is simply used to disable the Send button until the send has completed.  It has no effect on the networking, just on the GUI.
[5] The updateControls method will handle control state.  This is purely application-specific.

Handle Notifications of Packets Received

ON_MESSAGE(UWM_NETWORK_DATA, OnNetworkData)

LRESULT CAsyncClientDlg::OnNetworkData(WPARAM wParam, LPARAM)
   {
    CByteArray * data = (CByteArray *)wParam;         // [1]
    CString s = ConvertReceivedDataToString(*data);   // [2]
    delete data;                                      // [3]
    m_Sending = FALSE;                                // [4]
    updateControls();                                 // [5]
    return (LRESULT)0;                                // [6]
   }
[1] The specifcation is that WPARAM is a CByteArray *.  It is cast to the correct pointer type.
[2] The ConvertReceivedDataToString function takes a CByteArray & (note the use of the dereference operator to achieve this) and returns a CString.  If the UTF-8 encoding specified characters that have no representation in the range U0000..U00FF, and the application is ANSI, the string may contain '?' characters to represent the untranslatable characters.  If the application is a Unicode app, the translation will be correct.

Note that until this point, we have not cared if the data being transmitted was binary or string data; the network layer does not care, either.  Only at this level, the application level, is any interpretation placed on the byte stream received.

[3] After the string has been converted, the incoming data array can be discarded. 
[4] This clears the "packet being sent" flag so that any GUI components that were disabled during the send can be re-enabled.
[5] The updateControls method will handle control state.  This is purely application-specific.
[6] Because this is a window-oriented message, its return type is LRESULT.  However, because the message was sent via PostMessage, there is no one to receive the return value.  So, by default, 0 is returned.

Handle notifications of packet transmission completions

ON_MESSAGE(UWM_SEND_COMPLETE, OnSendComplete)

This is optional, and in the client, this is not a matter of concern.  Since the message posted contains no heap pointers, it can simply be ignored.  So this Message Map entry and its handler function are not present in this particular application.

Handle notifications of errors

ON_MESSAGE(UWM_NETWORK_ERROR, OnNetworkError)
LRESULT CAsyncClientDlg::OnNetworkError(WPARAM wParam, LPARAM)
    {
     DWORD err = (DWORD)wParam;                                  // [1]
     CString msg = ErrorString(err);                             // [2]
     c_Log.AddString(msg);                                       // [3]
     return 0;                                                   // [4]
    } // CAsyncClientDlg::OnNetworkError
[1] The specifcation is that WPARAM is a DWORD.  It is cast to the correct type.  In a 64-bit app, without this cast there would be a warning about integer truncation.
[2] The ErrorString function converts the error number to a printable string.  The particular variant that I'm using understands how to decode WinSock errors as well.
[3] This adds the string to a list box, which is my choice of control for displaying what is happening.
[4] Because this is a window-oriented message, its return type is LRESULT.  However, because the message was sent via PostMessage, there is no one to receive the return value.  So, by default, 0 is returned.

Shut down the thread

See the example of CAsyncServerDlg::OnClose.  The code for the client and the server are identical except for the class names of the functions.

Recognize when the shutdown has completed

This is similar to the CAsyncServerDlg::OnConnectionClose handler, but is a bit simpler since the client has only one thread.

 ON_MESSAGE(UWM_CONNECTIONCLOSE, OnConnectionClose)

LRESULT CAsyncClientDlg::OnConnectionClose(WPARAM, LPARAM lParam) // [1]
   {
    TRACE(_T("%s: CAsyncClientDlg::OnConnectionClose\n"), AfxGetApp()->m_pszAppName);
    m_pClientThread = NULL;                                       // [2]
    m_Connected = FALSE;                                          // [3]
    m_Connecting = FALSE;                                         // [4]

    updateControls();                                             // [5]
    if(m_MainWndIsClosing)                                        // [6]
       PostMessage(WM_CLOSE);                                     // [7]
    return 0;
   }
[1] The LPARAM is the thread ID of the thread that has closed.  The application might use this if it is maintaining more than one server connection, but for this application it is ignored.
[2] The thread will be terminating asynchronously, if it hasn't already, so the pointer to it will soon become, or already has become, meaningless.  By setting the pointer to NULL, any erroneous uses of the pointer should quickly show up.
[3] This socket is no longer connected.  Any flags that determine what controls should be enabled or disabled are cleared.
[4] If the thread terminated because of a connection failure, then it may not have been connected, but the connection that was in progress as terminated.
[5] The updateControls method will handle control state.  This is purely application-specific.
[6] In this application, there is only one connection to a server.  So if the thread terminated because an application shutdown initiated the thread termination, the close operation is reinitiated.  In this case, it will find there is no active thread, and the OnClose handler will exit the application.
[7] Because this is a window-oriented message, its return type is LRESULT.  However, because the message was sent via PostMessage, there is no one to receive the return value.  So, by default, 0 is returned.

CClientSocket::ProcessReceive

This virtual method handles the received data.  Since the choice was made to put the sockets in separate threads, this is called in the context of the thread that owns the socket.  Ordinarily, it might do a substantial amount of processing, which is why the choice was made to use a thread implementation in the first place.  However, for this demonstration project, it is fairly trivial.  Consider the SendDataToOwner call as being a placeholder for some potentially lengthy computation.

void CClientSocket::ProcessReceive(CByteArray & data)
    {
     SendDataToOwner(data);
    } // CClientSocket::ProcessReceive

Control Management

Client is not connected.  No host name typed Client is not connected.  Has host name.  Connect button enabled. Client is connecting.  Connect button, disconnected button, other controls disabled. (Note I had to use an IP address with no server so I had a long enough timeout to capture this image...) Client is connected.  Disconnect button is enabled.  Send button is disabled because no text has been typed.  Server and Server Port values are Read-Only but enabled.
Client is connected.  Send button is enabled because text has been typed into the Send window. A string has been sent and echoed back with >>><<< surrounding it. 20 strings of length 8,192 have been sent Client has disconnected from server

CAsyncClientDlg::updateControls

void CAsyncClientDlg::updateControls()                         // [1]
    {
     CString port;                                             // [2]
     c_Port.GetWindowText(port);                               // [3]

     UINT portnum;                                             // [4]
     portnum = _tcstoul(port, NULL, 0);                        // [5]
     
     CString srv;                                              // [6]
     c_ServerName.GetWindowText(srv);                          // [7]
     srv.TrimLeft();                                           // [8]
     srv.TrimRight();                                          // [9]
     
     //----------------
     // [_Connect_]
     //----------------
     // This is to illustrate how to implement a constraint.
     // I have arbitrarily chosen to not allow the selection
     // of a port in the Reserved or Well-Known-Ports area
     // (0..1023)
     c_Connect.EnableWindow(!m_Connecting &&                   // [10]
                            !m_Connected &&                    // [11]
                            !srv.IsEmpty() &&                  // [12]
                            portnum > 1023 &&                  // [13]
                            portnum <= 65535);                 // [14]
     //----------------
     // [_Disconnect_]
     //----------------
     c_Disconnect.EnableWindow(m_Connected);                   // [15]

     //----------------
     // Server [ name ]
     //----------------
     BOOL changeable = !m_Connected && !m_Connecting;          // [16]
     c_ServerName.SetReadOnly(!changeable);                    // [17]
     x_ServerName.EnableWindow(changeable);                    // [18]

     //----------------
     // Port [ number ]
     //----------------
     c_Port.SetReadOnly(!changeable);                          // [19]
     x_Port.EnableWindow(changeable);                          // 

     //---------------- 
     // [_Send_]
     //----------------
     CString msg;                                              
     c_Message.GetWindowText(msg);                             // [20]
     c_Send.EnableWindow(m_Connected &&                        // [21]
                         !m_Sending &&                         // [22]
                         !msg.IsEmpty());                      // [23]
    } // CAsyncClientDlg::updateControls         
[1] My style of doing dialogs is to write a single, centralized handler for all control state management.  Anything that changes a state that would affect the controls simply calls updateControls and all controls are properly computed.  I describe this in a separate essay.
[2] The port is stored as a string representing an integer.
[3] The text of the port number is retrieved.
[4] The port number is a UINT
[5] The _tcstoul converts it to an integer value.  By specifying a radix of 0, the function will also allow hex values like 0xC001 to be used.
[6] The server name is a string.  By using MFC (instead of GetDlgItemText) we do not need to worry about buffer sizes, buffer overflow, etc.
[7] The text is retrieved from the server name control.
[8] Leading spaces are trimmed from the server name.
[9] Trailing spaces are trimmed from the server name.  Note that in VS.NET and later, I could have written srv.Trim() to trim both leading and trailing spaces, but this code is intended to be compatible with VS6 as well, which does not have a Trim() method.
[10] The Connect button is disabled.if we are in the middle of performing a connection.
[11] The Connect button is disabled if we are already connected.
[12] The Connect button is disabled if the name of the server is the empty string.
[13] The Connect button is disabled if the port number is <= 1023 (in the "well-known port" region).  This was an arbitrary decision on my part to demonstrate how a range limit could be imposed.
[14] The Connect button is disabled if the port number is >65535.  Note that since it is a UINT, the user could type in a port number like "75000" which would be illegal.  Furthermore, if the value 65561 were typed in, because only the low-order 16 bits are used, this would allow the use of port 25, the
[15] The Disconnect button is enabled only if the client is connected.
[16] The server name cannot be changed if we are either in the midst of a connection being made, or we are already connected. 
[17] Disabling the input controls is not quite the correct solution to the problem.  I want the user to be able to copy the value, for example, to paste it into another instance of a client.  If I simply disable the control, this is not possible.  But by calling SetReadOnly, I merely disable the ability to change the text, without disabling the ability to select it, copy it, scroll it, etc.
[18] When a control is disabled, its caption should be disabled; this is a pleasing aesthetic.  My captions are always designated with names that start with x_ and have the same name as their corresponding control variable.
[19] For the same reason I want to make the server name unchangeable but copyable, I want to make the port number unchangeable but copyable.  The code here calls SetReadOnly on the edit control and EnableWindow on its caption.
[20] Retrieve the text from the sample message.  Note that if I wanted to disallow blank messages (messages containging only spaces) I would then follow this with a TrimLeft() operation.  If the string is only whitespace, trimming the leading spaces will result in an empty string.  This will mean that the testing for a non-empty send string would fail.
[21] The Send button is disabled if we are not connected
[22] The Send button is disabled if we are in the middle of a send.  This was an arbitrary design decision on my part.  The messaging and transport layers will work perfectly correctly if the Send button is clicked while a Send is active.  I just chose to do this to demonstrate how it can be done.
[23] The Send button is disabled if there is no text to send.

Server Side

class CAsyncServerDlg {
   protected:
        CListensoc m_listensoc;
   ...
};

CAsyncServerDlg::CreateListener

This code illustrates how a listening socket of class CListenSoc is created and initialized. 

     if(!m_listensoc.Create(portnum))                                         // [1]
        { /* failed to create */
         DWORD err = ::GetLastError();                                        // [2]
         ... format the error message and use PostMessage(UWM_INFO...)        // [3]
         ... to send it to this window
         return FALSE;                                                        // [4]
        } /* failed to create */

     ... report success

     m_listensoc.SetTarget(this);                                             // [5]

     m_listensoc.Listen();                                                    // [6]
     return TRUE;                                                             // [7]

[1] The value portnum has been initialized to hold the port number that is to be bound for this server. The socket is created.  When an explicit port number is given as an argument, this is a server socket and the port number is bound
[2] If it failed, capture the reason
[3] Details of formatting the error message and posting it are omitted
[4] return FALSE  to indicate failure.
[5] The target to which messages from the thread will be posted is set.  It must be set before the Listen method is called.
[6] The Listen method of CAsyncSocket will now be able to send OnAccept notifications.
[7] return TRUE to indicate succes

download.gif (1234 bytes)

 

 

[Dividing Line Image]

The views expressed in these essays are those of the author, and in no way represent, nor are they endorsed by, Microsoft.

Send mail to newcomer@flounder.com with questions or comments about this web site.
Copyright © 2007 Joseph M. Newcomer/FlounderCraft Ltd.  All Rights Reserved.
Last modified: May 14, 2011