CAsyncSocket message handling
neilsolent
Socket Notification Sink
Blah blah write at 0x00000004
Unhandled exception MFCN42D.DLL 0xc0000005 (access violation)
The app listens on a TCP port using instances of a CAsyncSocket-
derived class. When a connection is accepted
but later rejected (if SSL handshake failed for example) the socket is
closed. After this point, the listening
loop is posted a message which seems to lead to the crash:
MSG msg;
while (GetMessage(&msg, NULL, 0, 0) == TRUE)
{
LOGWARN "Received msg: [message=%u, hwnd=%d, wParam=%u, lParam=%u,
x=%u, y=%u]",
msg.message, msg.hwnd, msg.wParam, msg.lParam, msg.pt.x,
msg.pt.y);
..
..
TranslateMessage(&msg);
DispatchMessage(&msg); // <- Line 2688
}
The last message received before the crash is logged as:
Received msg: [message=883, hwnd=2556196, wParam=524, lParam=32,
x=869, y=828]
The stack backtrace shows:
MFCN42D! 5f6052b8()
MFCN42D! 5f605ab6()
MFCN42D! 5f60633f()
MFC42D! 5f43177c()
MFC42D! 5f4310b8()
MFC42D! 5f42ec09()
MFC42D! 5f42f0f5()
MFC42D! 5f49265d()
USER32! 7739c3b7()
USER32! 7739c484()
USER32! 7739c73c()
USER32! 7738e406()
_tcomx(void * 0x001d011a) line 2688 + 12 bytes
KERNEL32! 77e66063()
I don't *know* what message 883 (0x0373) is - I have not posted it
myself, and it is not mentioned in any header files I can find. I
*assume* it is how the callbacks like CAsyncSocket::OnReceive() are
implemented.
Hence - the crash is probably because I am processing a callback on a
socket that has already been deleted. Question is - how can I fix the
code? I assume the callback messages were already in the queue before
I deleted the socket..
I could do some lookup using the wParam member of the MSG (which seems
to match the socket handle) - seems a bit messy though. Hopefully
someone has already got round this in a neater way ..
Stephen Myers
You are correct, 0x373 is a private message used by the MFC socket
handling. wParam indicates the socket and lParam is a bit mask
indicating the cause (Read, Accept, Close etc). FD_CLOSE is the
message, so handling OnClose before deletion should do it.
Steve
neilsolent
> You are correct, 0x373 is a private message used by the MFC socket
> handling. wParam indicates the socket and lParam is a bit mask
> indicating the cause (Read, Accept, Close etc). FD_CLOSE is the
> message, so handling OnClose before deletion should do it.
Thanks for the info.
In my case I can't rely on OnClose() arriving. Sometimes I want to
delete a socket for some other reason than Windows telling me to!
I have got this working now - these are my tips to avoid this problem:
1. First call AsyncSelect(0) on any CAsyncSocket that is to be
deleted. This quiesces it (prevents any more callbacks being queued).
2. Right after the AsyncSelect(0), post a message to the queue of the
thread that owns the socket, instructing it to delete the socket. This
will ensure that no callback messages are received after the socket
has been deleted.
3. (Obviously) add code to handle the DELETE_SOCKET message you posted
4. Do not call Close() on any socket before the CAsyncSocket
destructor is called - to ensure the handle is not reused before the
old CAsyncSocket has been destroyed.
Joseph M. Newcomer
On Wed, 16 Sep 2009 00:54:51 -0700 (PDT), neilsolent <n...@solenttechnology.co.uk> wrote:
>My app is crashing with the following pop-up in a certain situation:
>
>Socket Notification Sink
>Blah blah write at 0x00000004
>Unhandled exception MFCN42D.DLL 0xc0000005 (access violation)
****
This is probably caused by the fact that you have received a notification after managing
to get a slock deleted from the handle map.
*****
>
>The app listens on a TCP port using instances of a CAsyncSocket-
>derived class. When a connection is accepted
>but later rejected (if SSL handshake failed for example) the socket is
>closed. After this point, the listening
>loop is posted a message which seems to lead to the crash:
****
Sounds about right. Where, exactly is this code? I cannot find these lines anywhere in
the MFC runtime. I specifically looked in the VS98 MFC source directory and could not
find them.
Note that if this is your code, then your code is erroneous, because it should not have
anything like this in it.
You say it is line 2688. I am having an error line 207 in my code, what's wrong with it?
Sorry, but a line number without a file name is pretty useless, particularly when the code
is not part of the MFC library.
*****
Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
neilsolent
> Sounds about right. Where, exactly is this code? I cannot find these lines anywhere in
> the MFC runtime. I specifically looked in the VS98 MFC source directory and could not
> find them.
Joseph,
The code was in the first post, line 2688 was clearly marked:
MSG msg;
while (GetMessage(&msg, NULL, 0, 0) == TRUE)
{
LOGWARN "Received msg: [message=%u, hwnd=%d,
wParam=%u, lParam=%u,
x=%u, y=%u]",
msg.message, msg.hwnd, msg.wParam,
msg.lParam, msg.pt.x,
msg.pt.y);
..
..
TranslateMessage(&msg);
DispatchMessage(&msg); // <- Line
2688
}
.. hence the last call from my code is DispatchMessage() - after that
the code is in Microsoft's libraries.
> Note that if this is your code, then your code is erroneous, because it should not have
> anything like this in it.
Shouldn't have anything like what in it?
Are you saying I shouldn't use DispatchMessage()?
Why not, and how else do the MFC callbacks get processed ?
Joseph M. Newcomer
of MFC, and therefore, since you give the code completely out of context, it is
questionable what it is doing or even why it exists.
If it is part of your code, you have said nothing that would suggest where it is or why it
exists.
joe
Joseph M. Newcomer
code for doing so. If so, it is highly suspect code. The correct way to run a socket in
a secondary thread is to create a UI thread to handle it; in general, you can have high
confidence that if you are writing a message pump in MFC, you are doing something very
wrong.
joe
neilsolent
> I know you pointed out that it was line 2688. But in WHAT FILE?
The name of the file is of no importance. The stack trace refers to a
line 2688. Hence I pointed to the line 2688 of my code.
> This is clearly not part
> of MFC, and therefore, since you give the code completely out of context, it is
> questionable what it is doing or even why it exists.
> If it is part of your code, you have said nothing that would suggest where it is or why it
> exists.
The context is as stated in the first post
neilsolent
> I'm wondering if you wanted to run an async socket in a separate thread, and this is your
> code for doing so. If so, it is highly suspect code. The correct way to run a socket in
> a secondary thread is to create a UI thread to handle it; in general, you can have high
> confidence that if you are writing a message pump in MFC, you are doing something very
> wrong.
> joe
Yes I am running an async socket in a "separate" thread. Why is it
highly suspect code?
This code now works fine. I think it is clear what the problem was and
how I solved it from my posts.
My app does not have a user interface.
I don't mind people making negative remarks - but please give a proper
explanation of why you think what I am doing won't work.
Stephen Myers
The UI thread design supplies the message pump for you. The name is
unfortunate. If you replace the name UI Thread with Thread with message
pump it helps.
The biggest problem with the code is that you seem to be doing it the
hard way. I would use the OnClose() override to delete the socket. If
you need to close the socket at other times, Close() is easy. Using a
UI thread leaves you with a small amount of code in InitInstance and the
a few message handlers.
Steve
Joseph M. Newcomer
error are very high, and as you observed, you have an error. The correct approach to this
problem is to use a UI thread, not hand-create a message pump in a worker thread.
The issue that can arise is a synchronization error in the lifetime of the local objects
in the per-thread message map. Writing your own code has several nasty implications,
including the fact that you are not doing any cleanup of temporary MFC objects that might
be created. So you can develop storage leaks.
Bottom line: if you want to handle sockets in a separate thread, use a UI thread, don't
try to simulate what you think the message pump might be doing (in your case, you got it
wrong, and as long as you leave that code there, the message pump is wrong, because it
does not match the expected behavior of an MFC message pump)
It is a negative comment because the code is wrong. You may think it is working, but in
fact it is missing critical pieces which the MFC environment expects.
joe
Joseph M. Newcomer
and I wrote my own message pump", which is the crucial piece of information. Otherwise,
the expectation is that you have hit some error in the MFC runtime.
joe
neilsolent
OK. The issue here was with the timing of deleting a CAsyncSocket-
derived object. How does the UI thread model handle that any better?
At what point would you delete the object in the UI thread?
Joseph M. Newcomer
or OnClose (incidating the socket has been closed). Only after the OnClose can you be
sure that the socket is no longer required, and it can be deleted. You did not establish
the basis for how you closed the socket, and therefore your code was clearly dispatching
to the (now nonexistent) window for the socket, hence the failure. Note that the
potential accumulation of garbage temporary objects remains a problem.
You have to show the ways in which you manipulate the socket; you cannot analyze a problem
like this just by showing a message pump. You showed a piece of code completely out of
context, with no explanation, so it was impossible to analyze what was there; had you
stated the problem (which should have been obvious once you looked at the stack backtrace
and saw what was going wrong) with the correct amount of detail, then the correct solution
would have been obvious.
Note that I had to rewrite the horrible Microsoft example for multithreaded sockets, which
got threading wrong, sockets wrong, and synchronization wrong, and I did this without
having to write a message pump.
If you do not understand how an MFC message pump works (not just an arbitrary message
pump; an MFC message pump) then trying to simulate it will produce erroneous results. If
your thread does not closely resemble CWinThread::Run and implement the same chain of
events, then it is incorrect. That OnIdle call is critical.
(Note: I have already proven your code is wrong; you cannot defend it based on the fact
that you like it, or think that it works, or once read about what a message pump in a
non-MFC app looked like in a book or example. It doesn't work. It presents the illusion
that it works. Delivering products based on illusory correctness is not a robust solution
strategy. Using a UI thread creates a correct solution, which actually IS correct.)
joe
neilsolent
> or OnClose (incidating the socket has been closed). Only after the OnClose can you be
> sure that the socket is no longer required, and it can be deleted. You did not establish
> the basis for how you closed the socket, and therefore your code was clearly dispatching
> to the (now nonexistent) window for the socket, hence the failure. Note that the
> potential accumulation of garbage temporary objects remains a problem.
I also need to close sockets on demand, rather than passively (in
response to an OnClose or whatever).
The peer may not want to close or disconnect, so I don't think I can
rely on the message coming in.
> You have to show the ways in which you manipulate the socket; you cannot analyze a problem
> like this just by showing a message pump. You showed a piece of code completely out of
> context, with no explanation, so it was impossible to analyze what was there; had you
> stated the problem (which should have been obvious once you looked at the stack backtrace
> and saw what was going wrong) with the correct amount of detail, then the correct solution
> would have been obvious.
Well, it's always obvious when you aleady have been given the answer.
Without the answer, I didn't know which part of the code was
important.
> If you do not understand how an MFC message pump works (not just an arbitrary message
> pump; an MFC message pump) then trying to simulate it will produce erroneous results. If
> your thread does not closely resemble CWinThread::Run and implement the same chain of
> events, then it is incorrect. That OnIdle call is critical.
>
> (Note: I have already proven your code is wrong; you cannot defend it based on the fact
> that you like it, or think that it works, or once read about what a message pump in a
> non-MFC app looked like in a book or example. It doesn't work. It presents the illusion
> that it works. Delivering products based on illusory correctness is not a robust solution
> strategy. Using a UI thread creates a correct solution, which actually IS correct.)
> joe
>
How did you prove the code is wrong?
Joseph M. Newcomer
****
By showing it failed to provide proper OnIdle handling, essential for an MFC-based thread
that might create temporary objects. And note that a thread such as the one you have that
requires a handle map (to handle the socket) requires correct cleanup be maintained.
****
neilsolent
> that might create temporary objects. And note that a thread such as the one you have that
> requires a handle map (to handle the socket) requires correct cleanup be maintained.
That is not a proof, that is an assertion.
I can add OnIde handling to the message loop if it is necessary.
I can't change it to an MFC-style app though as it does a lot more
than just process CAsyncSocket objects, and it would be very difficult
to get the other bits to work as well.
In a complex development, no one programming model is a perfect fit,
so you comprimise.
Stephen Myers
neilsolent wrote:
>> There will be a call on either OnDisconnect (indicating the remote site had disconnected)
>> or OnClose (incidating the socket has been closed). Only after the OnClose can you be
>> sure that the socket is no longer required, and it can be deleted. You did not establish
>> the basis for how you closed the socket, and therefore your code was clearly dispatching
>> to the (now nonexistent) window for the socket, hence the failure. Note that the
>> potential accumulation of garbage temporary objects remains a problem.
>
> I also need to close sockets on demand, rather than passively (in
> response to an OnClose or whatever).
> The peer may not want to close or disconnect, so I don't think I can
> rely on the message coming in.
Does calling the socket's Close() method satisfy your on demand requirement?
Steve
neilsolent
>
> Steve
Yes, Close() is the call - but the issue for me is knowing when you
can call it!
I don't believe you can call Close() whenever you like - as there
might be queued up notification messages (OnReceive() callbacks etc)
which will point at a bad handle once Close() has been called, and
hence lead to a crash like the one above. We have got into this
message-pump debate, but I think it is beside the point. It is not
clear how the MFC-style application and built-in message pump does
anything to get around this - you still can't close the socket on
demand (only when a Windows message tells you to).
Unless someone knows different ..
Joseph M. Newcomer
guarantee that potentially pending data has been flushed. Note that the CAsyncSocket
object cannot be actually deleted until the OnClose notification has been received.
See the discussion "Graceful shutdown, linger options, and socket closure" in the MSDN. It
does say "In fact, it has become a rather common programming practice to rely on [the fact
that the closesocket implicitly causes a shutdown sequence to occur if it has not already
happened] and to use closesocket to both initiate the shutdown sequence and deallocate the
socket handle"
As I read page 130 of Quinn & Shute ("Windows Socket Network Programming") they show a
diagram where a client first does a shutdown(), and waits for the FD_CLOSE notification to
arrive (OnClose), and only then close the handle. Surprisingly (or perhaps not so),
CAsyncSocket seems remarkably silent on when it is safe to actually delete the
CAsyncSocket object. But the diagram suggests that the sequence is to handle CAsyncSocket
by a sequence
Call CAsyncSocket::Shutdown()
In OnClose, call CAsyncSocket::Close()
delete the object.
In reading the code in sockcore.cpp, it appears that after an OnXXX event, the object is
no longer used. There is also a comment about looking up already-closed handles in the
handle map. But it appears that the above sequence is what is expected.
The MSDN "Windows Sockets Sample List" is empty, except for a circular reference to the
Windows Sockets top-level topic.
joe
Stephen Myers
I won't say that I know, but that is exactly what I'd use. You may get
notifications after the Close call, but that's OK. I would expect that
OnClose() would be queued like any other socket message, and it would be
reasonable to delete the socket at that point. Any messages received
between your Close() call and OnClose() would have to be handled. This
should be straight forward as you decided to call Close in the 1st place.
Steve
Joseph M. Newcomer
the Close call, representing packets that were already in flight. The FD_CLOSE/OnClose
notification will be queued up behind all those. It otherwise appears that after the
OnClose that the object will never receive another message. The error that was originally
reported appears to be the result of deleting the object before the last message (the
FD_CLOSE notification) had been processed.
joe
neilsolent
> As far as I can tell, that is correct. There can be any number of messages received after
> the Close call, representing packets that were already in flight. The FD_CLOSE/OnClose
> notification will be queued up behind all those. It otherwise appears that after the
> OnClose that the object will never receive another message. The error that was originally
> reported appears to be the result of deleting the object before the last message (the
> FD_CLOSE notification) had been processed.
> joe
I wasn't 100% convinced that Windows would reliably send the OnClose()
notification, especially if I initiated the shutdown of the socket (I
thought you might only get the OnClose() when the peer initiated the
shutdown by closing it's end of the socket). Hence I went for calling
AsyncSelect(0) to stop any more messages of any kind coming in, then
posting my own message to the queue to request deletion (knowing this
message is gauranteed to come after any Windows notifications). I
suspect either way works.
Back on the topic of the message pump.. This project was originally
built in VC++ starting with the options "Win32 Console Application" ->
"An application that supports MFC" -
and the program It runs as a service. Was there a step I was supposed
to do to get the standard MFC message pump? I wasn't really trying to
go non-standard - just doing what I needed to do to make it work !
Joseph M. Newcomer
comes free.
joe
neilsolent
My app was created originially in VC++ 6.0 using the project wizard as
a "Console application which supports MFC". This doesn't pull in any
MFC message loop. Are you saying that such applications are invalid?
I looked into the OnIdle() processing MFC source. It looks to me like
it only does any action if you have GUI components - which my
application doesn't. Hence I assume I am OK to use my own message
loop.