IPC - Use After Free

[itayzafrir]

Describe the bug
Use after free in nni_list_node_remove leads to process crash.
This is a continuation of #1827 and #1831 which were closed.
#1834 did not fix the use after free issue - at least for the IPC.

Expected behavior
No user after free, process does not crash.

Actual Behavior
ipc_send_cb is called after the pipe has been already closed, causing use after free crash.

To Reproduce
Use branch from https://github.com/itayzafrir/nng/tree/itay/new-use-after-free and run the demo/reqrep.c executable as server and another one as client.

** Environment Details **

• NNG version: master branch
• Operating system and version: Windows 10
• Compiler and language used: C/C++ Clang
• Shared or static library: static

Additional context
Stack Trace of crash (same as before):

Looks like aio has already been freed/deleted.

Another point, sometimes the client process crashes with a segfault. I wasn't able to debug this yet but it does happen from time to time.

[gdamore]

There is still indeed a use after free here. I spent pretty much most of last weekend trying to debug it. It seems like our attempt to unregister the method from the callbacks is not quite complete, or Windows is issuing a second completion callback.

We should not have had any outstanding requests when this callback was fired, as we try to ensure that there are no such requests pending or likely to be queued when we call close.

I'll have another look in the next day or two -- maybe something will occur.

[alzix]

please see this post https://qualapps.blogspot.com/2010/05/understanding-readdirectorychangesw_19.html
the Shutting down section.
it seems the CancelIo does not reliably abort all future events...

I re-read the documentation for CancelIo, which makes the statement that "All I/O operations that are canceled complete with the error ERROR_OPERATION_ABORTED, and all completion notifications for the I/O operations occur normally." Decoded, this means that all Completion Routines will be called at least one final time after CancelIo is called.

perhaps we could add HasOverlappedIoCompleted busy wait loop.

[gdamore]

This is a good idea. Its pretty crummy how this works, but we can do it. It will make tear down a bit more expensive on Windows, but it is what it is.

[gdamore]

@alzix if you can have a look at PR #1848 I'd like to know if it helps. I'm having some trouble with my Windows dev environment right now, so I'm not sure I can trust my results.