GDALRaster Garbage Collection not working

[Daniel Wiesmann]

I noticed that when creating many GDALRasters in a single context, the memory allocated to GDALRasters does not get released properly. For instance, the following loop as an ever growing memory consumption:

from django.contrib.gis.gdal import GDALRaster
n = 1000
for x in range(n):
    rast = GDALRaster({
        'driver': 'MEM',
        'width': 500,
        'height': 500,
        'nr_of_bands': 1,
        'srid': 3086,
        'origin': (500000, 400000),
        'scale': (1, -1),
        'skew': (0, 0),
        'bands': [{
            'nodata_value': 10,
            'data': range(500**2)
        }],
    })

Adding rast.__del__() to the loop solves the problem, but I guess that should be automatic.

[Daniel Wiesmann]

Unfortunately this goes beyond my knowledge of how python works. I am happy to work on this if its productive, but would need help.

[Tim Graham]

I don't have much experience with garbage collection, but after doing some reading I think this is expected as there is no guarantee that the garbage collector will run in code like this. If you put that code in a function, is __del__() called when the function exits (i.e. local variables should be garbage collected at that point)?

[Claude Paroz]

It might be related to bands. I made some tests, and as soon as I access self.bands, the raster in not garbage collected. Perhaps because there is a circular reference between raster (bands cached property) and bands (band.source). That's a guess for now...

[Claude Paroz]

Allow GDALRaster garbage collection

[Claude Paroz]

Daniel, could you test the attached patch and see if that changes anything in your experiments?

[Daniel Wiesmann]

Thanks Claude, I can confirm that your patch solves the problem for my case. I just tried the patch on both the loop posted above and the use case where I spotted the problem in the first place. The memory usage stayed flat in both runs.

[Daniel Wiesmann]

I just noticed that I started getting segmentation faults when accessing the GDALBand data method after applying the patch. I will have a closer look at what is going on there.

[Claude Paroz]

Yes, you'll get segmentation faults if you use a band while its source has been garbage collected. The patch needs improvements to safeguard against this, if possible.

[Daniel Wiesmann]

This test leads to a segfault on my system.

[Daniel Wiesmann]

I tracked down the segmentation fault I was getting, see the diff I attached. Somehow accessing the data directly from a queryset without creating intermediate objects gives me a segmentation fault. With an intermediate object it does not.

[Claude Paroz]

Here's another possible solution: ​https://github.com/django/django/pull/4971

That is stop to cache GDALRaster.bands, which is the cause of the other side of the circular reference.

[Daniel Wiesmann]

That looks fine to me, I think most rasters have only a few bands.

Maybe we can add a method to get only one specific band by index? Like this single bands can be accessed efficiently. Something along the lines of:

def get_band(self, index):
    if index < 0 or index >= capi.get_ds_raster_count(self._ptr):
        raise GDALException('Band index out of range.')
    return GDALBand(self, index)

[Claude Paroz]

Your wishes are my orders :-) Please check the latest PR version (while still checking the initial memory issue).

[Daniel Wiesmann]

Thanks Claude!

I just ran your patch through the loop and applied use case and it worked perfectly, including low memory use.

The issues that came up through this ticket has made me realize that it could be important to only instantiate specific bands (some time series rasters might have lots of bands, its is not common, but happens). That's why I asked for this...

[Claude Paroz <claude@…>]

In d72f8862:

Fixed #25072 -- Prevented GDALRaster memory to be uncollectable

Setting GDALRaster.bands as a cached property was creating a circular
reference with objects having del methods, which means the memory
could never be freed.
Thanks Daniel Wiesmann for the report and test, and Tim Graham for the review.