Cleaning up client session arch to stop frag crash. #56
Conversation
| while (transport->status() == qtransport::TransportStatus::Connecting) { | ||
| std::ostringstream log_msg; | ||
| log_msg << "Waiting for client to be ready, got: " | ||
| << int(transport->status()); | ||
| logger.log(qtransport::LogLevel::info, log_msg.str()); | ||
| while (!stopping && client_status == ClientStatus::CONNECTING) { | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(100)); | ||
| } | ||
|
|
||
| client_status = ClientStatus::READY; | ||
| if (stopping) { | ||
| log_handler.log(qtransport::LogLevel::info, | ||
| (std::ostringstream() << "Cancelling connecting session " |
There was a problem hiding this comment.
Though we try hard, picoquic (and even harder, UDP) decide that they don't want to be stopped, and continue on without a care. So fully stopping this when we want to leave a call if we didn't connect is a bit of an issue.
There was a problem hiding this comment.
Both UDP and QUIC transports should be completely stopped if destructed. There's no shutdown method in the transport, which we should add... in a different PR unless you want to add it to this one.
There was a problem hiding this comment.
Created #57 for this.
| while (transport->status() == qtransport::TransportStatus::Connecting) { | ||
| std::ostringstream log_msg; | ||
| log_msg << "Waiting for client to be ready, got: " | ||
| << int(transport->status()); | ||
| logger.log(qtransport::LogLevel::info, log_msg.str()); | ||
| while (!stopping && client_status == ClientStatus::CONNECTING) { | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(100)); | ||
| } | ||
|
|
||
| client_status = ClientStatus::READY; | ||
| if (stopping) { | ||
| log_handler.log(qtransport::LogLevel::info, | ||
| (std::ostringstream() << "Cancelling connecting session " |
There was a problem hiding this comment.
Both UDP and QUIC transports should be completely stopped if destructed. There's no shutdown method in the transport, which we should add... in a different PR unless you want to add it to this one.
A bit of context here: The crashes around
notify_pub_fragment(an example shown in Quicr/quicr-mac#152) seem to be caused by a race with deleting entries from thesub_delegatesmap, and trying to access the whole map in the notify.A few things here:
subscribe_statemap was being fully emptied by both theQuicrClientRawSessionandQuicrClientTransportDelegatedestructors, which doesn't make a whole lot of sense.The changes here: