[Spice-devel] leaking drawables

[Alexander Larsson]

On Thu, 2010-09-09 at 09:31 +0200, Gerd Hoffmann wrote:
> 
> There is one special thing in the release ring usage though.  As you 
> know qxl builds up a linked lists there, where the heads are in the 
> ring.  qxl does also store the head of the list which it currently 
> builds into the ring, but doesn't bump prod (yet) so the guest will not 
> grab it.  Which needs one extra ring entry.  Because of that the IS_FULL 
> macro simply doesn't work for the release ring and the check is open coded.
> 
> So, the old code checks whenever prod - cons + 1 != num_items.  Lets 
> assume the guest didn't take out any entries yet, so cons is 0.  When 
> prod is 7 the check fails and qxl will stop pushing stuff to the ring 
> due to being full.  Lets have a look at the prod == 6 case:
> 
>    * The extra entry (ring[7%8]) holds the head for the list qxl
>      is building).
>    * qxl pushes that ring item to the guest (SPICE_RING_PUSH),
>      prod is 7 now.
>    * qxl gets a pointer to the next entry (SPICE_RING_PROD_ITEM),
>      which is ring[8%8].
>    * qxl zero-initializes the ring entry as new list head, thereby
>      overriding ring[0], which isn't yet consumed by the guest.
> 
> Correct?

No, that doesn't seem correct to me.

Initially, both prod an cons are zero, and has 
 num items = prod - cons = 0 - 0 = 0

Then we release an item (interface_release_resource). This starts
writing to the current producer item SPICE_RING_PROD_ITEM(ring, item),
either initializing it or appending to the list. In this case prod is
zero, so we're writing to ring->items[0].

Then we push this, which increases prod to 1 and then initializes the
new temp item (at 1).

Eventually we reach the case where prod is 6 (and lets say cons is still
0). We put stuff in item 6, then SPICE_RING_PUSH increasing prod to 7,
initialize item 7. No overwrite here. Then when prod is 7 we keep
appending to the item 7 list, but never push it because prod - cons + 1
= 7 - 0 + 1 = 8 == num_items.