Opened 4 months ago

Closed 4 months ago

Last modified 2 weeks ago

#29508 closed defect (fixed)

KIST does not check the right channel's sched_heap_idx when readding channels

Reported by: pastly Owned by:
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-sched, kist, 040-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

Link to the section of code as of right now: https://gitweb.torproject.org/tor.git/tree/src/core/or/scheduler_kist.c#n724

The buggy code

  /* Re-add any channels we need to */
  if (to_readd) {
    SMARTLIST_FOREACH_BEGIN(to_readd, channel_t *, readd_chan) {
      scheduler_set_channel_state(readd_chan, SCHED_CHAN_PENDING);
      if (!smartlist_contains(cp, readd_chan)) {
        if (!SCHED_BUG(chan->sched_heap_idx != -1, chan)) {
          /* XXXX Note that the check above is in theory redundant with
           * the smartlist_contains check.  But let's make sure we're
           * not messing anything up, and leave them both for now. */
          smartlist_pqueue_add(cp, scheduler_compare_channels,
                             offsetof(channel_t, sched_heap_idx), readd_chan);
        }
      }
    } SMARTLIST_FOREACH_END(readd_chan);
    smartlist_free(to_readd);
  }

The code wrapped inSCHED_BUG should be checking readd_chan not chan.

This has never been an issue in mainline-Tor because the scheduler never leaves its while loop with channels in channels_pending. But if you make changes to Tor's code that allow for the scheduler to leave its loop without emptying channels_pending, then this condition will often fail, which cumulates in tor ultimately seemingly forgetting about the channel and letting it sit idle.

Branch incoming.

Child Tickets

Change History (4)

comment:2 Changed 4 months ago by dgoulet

Keywords: tor-sched kist 040-backport added
Reviewer: dgoulet
Status: needs_reviewmerge_ready

lgtm; Here is a PR and 040 branch:

Branch: ticket29508_040_01
PR: https://github.com/torproject/tor/pull/704

Last edited 4 months ago by dgoulet (previous) (diff)

comment:3 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to 0.4.0 and forward!

comment:4 Changed 2 weeks ago by teor

Milestone: Tor: 0.4.1.x-finalTor: 0.4.0.x-final
Note: See TracTickets for help on using tickets.