Prev: Knit picking
Next: No source on specified on that location
From: Joseph M. Newcomer on 22 Sep 2009 08:14
See below...
On Tue, 22 Sep 2009 01:48:03 -0700 (PDT), neilsolent <n(a)solenttechnology.co.uk> 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.
>
>> 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?
****
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.
****
Joseph M. Newcomer [MVP]
email: newcomer(a)flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
From: neilsolent on 22 Sep 2009 08:24
> 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.

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.
From: Stephen Myers on 22 Sep 2009 12:32

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
>
>> 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?
From: neilsolent on 23 Sep 2009 08:05
> Does calling the socket's Close() method satisfy your on demand requirement?
>
> 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 ..
From: Joseph M. Newcomer on 23 Sep 2009 11:44
That is correct. The WinSock specification says that you MUST call Shutdown() first to
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
On Wed, 23 Sep 2009 05:05:19 -0700 (PDT), neilsolent <n(a)solenttechnology.co.uk> wrote:

>> Does calling the socket's Close() method satisfy your on demand requirement?
>>
>> 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 [MVP]
email: newcomer(a)flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: Knit picking
Next: No source on specified on that location