Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#5449 closed Bug (incomplete)

'have_all_hint' and 'have_none_hint' are currently broken

Reported by: cfpp2p Owned by: jordan
Priority: Normal Milestone: None Set
Component: libtransmission Version: 2.80
Severity: Normal Keywords:
Cc:

Description

Based on the backtrace at fixed ticket #5444 and Downstream ticket ​https://bugzilla.redhat.com/show_bug.cgi?id=989258 (please see backtrace there or attachment here as to not clutter this ticket)

In the backtrace we see

completion.c:320
        ret = <optimized out>
        n = 0
        pieces = {bits = 0x0, alloc_count = 0, bit_count = 0, true_count = 0, have_all_hint = true, have_none_hint = false}

showing

bit_count = 0, true_count = 0, have_all_hint = true, have_none_hint = false

With a magnet link (and with no metadata yet) bit_count = 0 and true_count = 0 is correct but have_all_hint = true should be false acording to typedef struct tr_bitfield (and have_none_hint should be true).

bitfield.h

typedef struct tr_bitfield
{
  uint8_t *  bits;
  size_t     alloc_count;

  size_t     bit_count;

  size_t     true_count;

  /* Special cases for when full or empty but we don't know the bitCount.
     This occurs when a magnet link's peers send have all / have none */
  bool       have_all_hint;
  bool       have_none_hint;
}
tr_bitfield;

have_all_hint is initialized to false.

have_all_hint = true and is only set true in one place and how did this become set true for a magnet link with no metadata yet?

void
tr_bitfieldSetHasAll (tr_bitfield * b)
{
  tr_bitfieldFreeArray (b);
  b->true_count = b->bit_count;
  b->have_all_hint = true;
  b->have_none_hint = false;

  assert (tr_bitfieldIsValid (b));
}

tr_cpHasAll() calls tr_bitfieldHasAll() :

static inline bool
tr_bitfieldHasAll (const tr_bitfield * b)
{
  return b->bit_count ? (b->true_count == b->bit_count) : b->have_all_hint;
}

when b->bit_count is 0 (false) then we use the have_all_hint which is set to true at this point which it shouldn't be for a magnet link with no metadata. The have_all_hint was created to deal with magnet links in the first place but is broken.

Noting that the reason for the have_all_hint is magnet links. This was introduced at https://trac.transmissionbt.com/changeset/12248#file2 and seems to have been working since v2.30 but now appears broken.

tellPeerWhatWeHave() was setting as true tr_cpHasAll (&msgs->torrent->completion) which should be false for a magnet link with no metadata.

The question in my mind is how this got broken in the first place and could it be realated to tickets such as #5372 and #5444

Attachments (1)

5444-copy-backtrace.txt (4.2 KB) - added by cfpp2p 9 years ago.

Download all attachments as: .zip

Change History (5)

Changed 9 years ago by cfpp2p

comment:1 Changed 9 years ago by jordan

  • Version changed from 2.81+ to 2.81

#5444 was reported against 2.80, so reassigning ticket's version

Last edited 9 years ago by jordan (previous) (diff)

comment:2 Changed 9 years ago by jordan

  • Version changed from 2.81 to 2.80

comment:3 Changed 9 years ago by jordan

  • Resolution set to incomplete
  • Status changed from new to closed

cfpp2p, I'm not able to trigger this bug. Steps to reproduce?

comment:4 Changed 9 years ago by x190

"have_all_hint = true and is only set true in one place and how did this become set true for a magnet link with no metadata yet?"

This issue may have its roots in #5168 and the resulting new swarm management code.

Last edited 9 years ago by x190 (previous) (diff)
Note: See TracTickets for help on using tickets.