make controller_run_result movable and expose it as start_result

[0x00002a]

Related to #106 and #107.

5874a5a implemented part of this but there is a subtle bug in it which shouldn't exist in this one. Feel free to close and/or cherry pick the move changes as the rest may be unwanted while #107 is open, but make sure to include 6456a16 as I accidentally removed the work guard when merging it with my local changes.

Also of note is the message in 6456a16, in which I discovered why we need a work guard for the io context.

[Tectu]

Well, that was some unfortunate timing :p

5874a5a implemented part of this but there is a subtle bug in it which shouldn't exist in this one.

Could you be more specific? I must be missing something obvious... Would love to hear about it.

[0x00002a]

5874a5a implemented part of this but there is a subtle bug in it which shouldn't exist in this one.

Could you be more specific? I must be missing something obvious... Would love to hear about it.

The threads spawned by controller_run_result capture this and then access the m_io_context through it. The issue here is that the function passed to the thread ctor only runs after the thread is spawned, meaning if the code to move the object is executed first, as is possible in the test case which does it on the next line (and which only happened for me when a debugger wasn't attached, that was fun), then the m_io_context being accessed through the this pointer was the one that had already been moved from and now pointed to nullptr. All in all I'm very glad I wrote a test case or I would never have caught that one :p

Also it doesn't check in run() for if the object has been moved from, meaning it could result in UB via nullptr deference there. In this I've thrown an exception but I'm not sure if that should be an assert

[Tectu]

The threads spawned by controller_run_result capture this and then access the m_io_context through it. The issue here is that the function passed to the thread ctor only runs after the thread is spawned, meaning if the code to move the object is executed first, as is possible in the test case which does it on the next line (and which only happened for me when a debugger wasn't attached, that was fun), then the m_io_context being accessed through the this pointer was the one that had already been moved from and now pointed to nullptr. All in all I'm very glad I wrote a test case or I would never have caught that one :p

Ouch... that one would definitely have made for some very, very unpleasant nights of debugging down the road. I am very glad that you caught that one... - Eagle eyes 😉

Also it doesn't check in run() for if the object has been moved from, meaning it could result in UB via nullptr deference there. In this I've thrown an exception but I'm not sure if that should be an assert

Now that was just a stupid oversight - should have taken the courtesy to get some sleep before committing this - sorry!

Not sure whether we should:

• Revert my changes so this becomes easily mergable
• Merge it by hand
• You updating your PR so it can be merged into main as part of the PR

[0x00002a]

Well I don't really have much more time to spend on this, and prolly won't for a few days unfortunately so it might be best to roll back 5874a5a since it isn't safe to move the object anyway and then merge this when it implements whatever is decided for #107, since otherwise its likely going to make the solution a breaking change.

I'm pretty sure I can just resolve conflicts by hand on github though by just not accepting 5874a5a when merging

[Tectu]

I'm a bit tight on time as well. I will revert my commit which introduced the bug you discovered/mentioned so that main and circle back to this later.

Thank you for all the time you invest into this. It's greatly appreciated :)

[0x00002a]

I've done the renames suggested in #107. I also renamed the header files and such, not sure if routing_context is the final name we want to go with, but start_result is now session instead and is exposed by both the client controller and server routing context

[Tectu]

Nice work!
Seems like we have some demo fixing to do.