#21027: tor_bug_occurred_(): Bug: src/or/entrynodes.c:816: entry_guard_add_to_sample_impl: Non-fatal assertion !(have_sampled_guard_with_id(gs, rsa_id_digest)) failed. (on Tor 0.3.0.0 -alpha-dev 8b75261b6dc341de) -------------------------------------------------+------------------------- Reporter: asn | Owner: nickm Type: defect | Status: | needs_revision Priority: High | Milestone: Tor: | 0.3.0.x-final Component: Core Tor/Tor | Version: Severity: Normal | Resolution: Keywords: tor-guard, tor-guards-revamp, | Actual Points: regression, review-group-16 | Parent ID: | Points: 1 Reviewer: | Sponsor: -------------------------------------------------+-------------------------
Comment (by asn): Hello, as I said also in IRC, I think that the provided patch addresses the issue. I tested this on my Tor Browser with the bridges from comment:17 and I verified that they are all treated as separate bridges indeed, and they get sampled to SAMPLED_GUARDS properly. That said, I agree with the comments of David about the code/comment quality: - The function documentation for `get_closest_configured_bridge_by_addr_port_digest()` seems to be incomplete and incorrect. What does `tolerate` mean? Does it actualy `tolerate a bridge with an unknown digest if the digest is provided`? I don't see this in the code. Also, if anything, the new function seems less tolerable than the old one in general. Also, is that the important difference of this new function for our purposes? IIUC, the important part for us is that it will only return a bridge if the addrport really matches; whereas the old `get_configured_bridge_by_addr_port_digest()` would return a bridge that matches the digest even if the addrport was completely different. Is that what you tried to express in the comment? Can we perhaps make it more clear? - Also, as David said, `get_closest_configured_bridge_by_addr_port_digest()` is kind of an ambiguous name. Can we clarify it? I couldn't think of a better name; these two functions are quite similar semantically with some small esoteric differences. Alternatively, could we merge these two functions together and use a func argument to disambiguate? All in all the patch seems good with regards to functionality, but perhaps we should take care of the above to improve code quality and maintainability. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/21027#comment:25> 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