Refactor handshake

[jens-diewald]

This is intended to simplify the handshake code. It should simplify reusing much of the handshake code in order to resolve #75 in a future PR.

I also changed a few small things i came across while working on this. All should be documented within the commit messages. I hope i did not miss any subtleties why async_handshake was a coroutine? I could not think of a good reason with the current code.

Feedback would be much appreciated :)

[laudrup]

@jens-diewald First of all thanks a lot for this PR. I really appreciate it.

Regarding the implementation of the handshake as a coroutine I had a lot of help from the nice guys from boost::beast, namely @vinniefalco and @madmongo1.

To be honest I can't remember all the details but I would rather avoid removing the coroutine part without understanding the full implications of that. We definitely don't want the async parts of the code to be blocking which is why a coroutine was needed in the first place as far as I remember.

Maybe we can get @vinniefalco and/or @madmongo1 to help a bit with this?

I definitely appreciate it if this could be simplified so I hope this can be merged in one way or another.

Thanks once again.

[jens-diewald]

@laudrup Whether async_handshake is implemented as a coroutine or not should not be a problem with regard to asynchronity by itself. (Of course i could have gotten something wrong.) Anyhow, i thought about this some more and i found, that if we remove the state member and enum, we can actually make use of the function being a coroutine. I have reverted the original change and added a new commit removing the state and making use of the coroutine instead. Maybe this is how it was originally intended?

I have then made another commit re-adding the comment regarding posting self on the first call to the function and a small simplification, so that the respective code is needed only once. I have also thought about this some more, but i am afraid i still do not really get it. I will reply to my comment on the code to elaborate my understanding so far.

[jens-diewald]

I have moved the self-posting logic into a seperate file, added a case distinction on BOOST_VERSION and added some comments. This is somewhat over-the-top, but i could not think of another reasonable way to document what is going on and how it should be done better in newer boost versions. For the case distinction i added boost 1.80 and boost 1.81 to the CI matrix.

@klemens-morgenstern, if you find the time to look at this and possibly confirm that it makes sense to you, i would appreciate that a lot.

@laudrup, sorry that this went somewhat off-topic from my originally intended changes. The self-posting stuff would have deserved a seperate issue. From my side, the PR could be merged like this now. (Assuming the tests go through and there are no other objections.)

[laudrup]

@jens-diewald So sorry for the late reply and thanks once again for your pull request.

I would definitely like this to work with boost versions earlier than the latest. A somewhat new version could be required. Assuming there's tests ensuring it works on different versions then adding it to the CI is definitely the right thing to do. Thanks for that.

@klemens-morgenstern Thanks a lot for your input here. It seems like you're a lot more experienced with the details of boost::asio than I am so if you think these changes make sense then I'll feel fairly confident merging them.

So if we get the tests to pass and @klemens-morgenstern approves then so will I, although I'll probably look through the code again a final time before merging.

Thanks a lot once again.

[laudrup]

Hi @jens-diewald,

So sorry for not having looked at this for ages. I haven't really looked at this library for quite some time.

Is this still something you'd like me to merge at some point?

As you might have noticed I've update the pipeline to test newer boost versions and made a new release so it might be a good time for me to revisit this.

The main issue I have with the change is that I would very much like someone else to have a look at it as well. If you're still interested I'll see if I can ping some relevant people.

Thanks a lot.

[jens-diewald]

Hi @laudrup,

yes I think this pull request does improve the code. I worked on this as a preparation of #78, but all the changes are independent of that issue. A second opinion regarding the post_self part would be nice.

[laudrup]

@madmongo1 has said he'll have a look at this when time permits.

Hope we can get this merged.