#30511: Remove OnIceComplete -------------------------------------+------------------------------ Reporter: arlolra | Owner: (none) Type: defect | Status: needs_review Priority: Low | Milestone: Component: Circumvention/Snowflake | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: | Sponsor: -------------------------------------+------------------------------
Comment (by arlolra): Bear in mind that this is just a port of an already merged patch (see the inline comments there), https://github.com/keroserene/snowflake/commit/c28c8ca489633aae2d9b9dbea0e781ca5e44cc66 `pc.CreateAnswer()` is a blocking operation, which is probably why it was put in a goroutine. However, `makePeerConnectionFromOffer` falsely assumed that if `OnIceComplete` was called, it was ready to move on to sending the answer. That setup a race between getting the local description in `sendAnswer` and setting it in the goroutine, which was never great. Not to mention that there was plenty of time for ice to fail after the gathering stage completed, which resulted in the deadlock. Since `makePeerConnectionFromOffer` always blocked waiting on the channels, it isn't a significant change to remove the goroutine since, as the comment in the commit states, // ... we need // SetLocalDescription(answer) to be called before sendAnswer It might be worth looking at whether `runSession` deserves to be called asynchronously, but that seems orthogonal to the change here. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30511#comment:4> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs