[tor-bugs] #25306 [Core Tor/Tor]: tor_assertion_failed_(): Bug: ../src/or/hs_service.c:1985: rotate_all_descriptors: Assertion service->desc_current failed; aborting.

[Tor Bug Tracker & Wiki]

#25306: tor_assertion_failed_(): Bug: ../src/or/hs_service.c:1985:
rotate_all_descriptors: Assertion service->desc_current failed; aborting.
------------------------------------------------+--------------------------
 Reporter:  cypherpunks                         |          Owner:  dgoulet
     Type:  defect                              |         Status:
                                                |  needs_review
 Priority:  Medium                              |      Milestone:  Tor:
                                                |  0.3.3.x-final
Component:  Core Tor/Tor                        |        Version:  Tor:
                                                |  0.3.3.2-alpha
 Severity:  Normal                              |     Resolution:
 Keywords:  tor-hs crash 033-must 032-backport  |  Actual Points:
Parent ID:                                      |         Points:
 Reviewer:  asn                                 |        Sponsor:
------------------------------------------------+--------------------------
Changes (by dgoulet):

 * status:  needs_revision => needs_review
 * reviewer:   => asn


Comment:

 Replying to [comment:16 asn]:
 > Two last things:
 > - Does the code that we added in this ticket allow the service to
 recover from this issue where it has no descriptors? That is, will it ever
 rebuild the needed descriptors and rotate them gracefully, or will it get
 permanently stuck in a broken state? My understanding from reading the
 code a bit is that the latter case will happen. Do we care about the
 latter if it happens?

 I think it will get stuck if `current` is NULL and `next` is not. We won't
 be able to rotate nor build a current one if no rotation.

 If both are NULL, it won't get stuck, new desc will be created. If only
 next is NULL, it won't get stuck as it will be built just after.

 Tbh, having a `current == NULL`, which I don't think is possible without
 also having a `next == NULL`, we have a very bad code flow issue. I think
 at that point, we might want the service to stop working properly so the
 operator can notice the BUG() and tell us. I've personally never ever seen
 this so if it happens (like this ticket maybe), it should be rare or if
 not, we'll catch it soon enough.

 > - In `rotate_all_descriptors()` let's add the `BUG` check inside
 `should_rotate_descriptors()`. I think that's conceptually the right place
 to do any check wrt rotating descriptors.

 See fixup commit: `4cad1376c6`

 Notice that I changed a bit the log message to print the desc pointer as
 in we do want to know which desc are NULL.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25306#comment:17>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online