reference counting for subimages isn't quite right

[rhl@…]

When a subimage is created, the reference count on parent->rawDataBuffer is incremented,
so it needs to be decremented by imageFree()

The helper routine imageSubset in psImageExtraction.c keeps a list of children which it
frees when a parent is freed; it therefore needs to increment their reference counters.
(parenthetically, shouldn't imageSubset be declared static? Or, if it has global linkage,
it should be renamed with a ps prefix).

Here's a patch to the two reference counter issues:

RCS file: /usr/local/cvs/repositories/pan-starrs/datasys/IPP/psLib/src/image/psImage.c,v
retrieving revision 1.65
diff -a -c -w -r1.65 psImage.c
* src/image/psImage.c 29 Apr 2005 02:25:10 -0000 1.65
--- src/image/psImage.c 6 May 2005 17:56:46 -0000
*
* 37,42
--- 37,43 ----

}

if (image->parent != NULL) {

+ psFree(image->parent->rawDataBuffer); we hold a reference to it

psArrayRemove(image->parent->children,image);
image->parent = NULL;

}

Index: src/image/psImageExtraction.c
===============================================================
====
RCS file: /usr/local/cvs/repositories/pan-starrs/datasys/IPP/psLib/src/image/psImageExtraction.c,v
retrieving revision 1.35
diff -a -c -w -r1.35 psImageExtraction.c
* src/image/psImageExtraction.c 8 Apr 2005 17:58:57 -0000

1.35

--- src/image/psImageExtraction.c 6 May 2005 17:56:46 -0000
*
* 122,128

} else {

n = children->n;

}

! children->data[n] = out;

children->n = n+1;
image->children = children; push back any change (esp. if children==NULL before)

--- 122,128 ----

} else {

n = children->n;

}

! children->data[n] = psMemIncrRefCounter(out);

children->n = n+1;
image->children = children; push back any change (esp. if children==NULL before)

[robert.desonia@…]

actually, isn't image->rawDataBuffer == image->parent->rawDataBuffer for a
child, and isn't that freed just a few lines down? I see no problem there.

Yes, imageSubset should be static; change made. Thanks for the catch.

[robert.desonia@…]

Incrementing the child psImage's reference count when storing it in the parents
list, at first glance, seems quite reasonable, but what that does is force the
child to be kept around until the parent is freed. In that case even if the
child is just freed, the reference count would still be 1, hence would not be
really destroyed. I don't think that is the desired behaviour.

I think it would be reasonable to not store the children in the parent at all
and allow the reference counting of the rawDataBuffer handle when the image data
can actually can be destroyed.

The fact that, currently, freeing the parent automagically frees the children is
probably confusing. I'd say it would be better to just orphan the children by
setting the child's parent to NULL, but I've started to use that check to
determine if the image is contigous in memory. If I made that change, I'd
probably would have to change the check for contiguous-ness. That probably
wouldn't be too hard, just need to check the location of the last pixel in the
first line with the first pixel of the next, or just add a boolean parameter in
the psImage structure.

-rdd

[jhoblitt]

Is this bug still relevant after the recent design changes to psImage?

[Paul Price]

Assuming this bug has been dealt with; it's old. Reopen if it's still relevant.