Useless allocation in CSVDeaccess

[bartoli]

I noticed a useless allocation in the code of the CPL.

Use case : I launch my application that uses GDAL on linux, then quit as soon as i can.

Some global destructor will call OGRCleanup -> OSRCleanup -> CSVDeaccess(NULL)

CSVDeaccess() will then call CPLGetTLS(CTLS_CSVTABLEPTR). This calls CPLGetTLSList. Inside that, pthread_getspecific() returns NULL because the TLS list has not been allocated yet.

As the pointer is NULL, CPLGetTLSLIst will allocate that list with VSIcalloc(). Then CPLGetTLS will look in that empty list and return a null pointer, and finally, CSVDeaccessINternal will return because the pointer is NULL.

Couldn't CSVDeaccess call a custom 'readonly' version of CPLGetTLS that will immediately return NULL if pthread_getspecific() instead of making GetTLSLIst allocate it (since we don't want to 'add' stuff in the TLS here, so no need to allocate anything for now), and then test that result BEFORE calling CSVDeaccessInternall to avoid a function call. This version of CPLGetTLS could also be used in other places i guess?

Moreover, i noticed this because i sometimes get a segmentation fault in the VSIcalloc in this scenario, probably because something that's needed for the malloc has already been deleted since i'm in global destructors

[Even Rouault]

It would be interesting if you could provide a test code that can help reproduce your issue. I'm wondering if your issue isn't linked to #4175 and the commits r25768, r25769, r25780 and r25781 (but that issue is fixed in 1.10, so your issue seems different)

[bartoli]

Well i'm not sure the segfault itself is an indication of a bug in GDAL. Here is the call stack i copied:

#0 0x0000003abea7046e in malloc_consolidate () from /lib64/libc.so.6 #1 0x0000003abea72a1a in _int_malloc () from /lib64/libc.so.6 #2 0x0000003abea7486d in calloc () from /lib64/libc.so.6 #3 0x00002ba8f072bddf in CPLGetTLSList () at cpl_multiproc.cpp:1552 #4 0x00002ba8f072be9b in CPLGetTLS (nIndex=-1093326368) at cpl_multiproc.cpp:1574 #5 0x00002ba8f07205fe in CSVDeaccess (pszFilename=0x0) at cpl_csv.cpp:231 #6 0x00002ba8f09b71e0 in OSRCleanup () at ogrspatialreference.cpp:7117 #7 0x00002ba8f091a656 in OGRCleanupAll () at ogrsfdriverregistrar.cpp:149 #8 0x00002ba8f045255f in do_global_dtors_aux () from /sdk/polaris4/linux64-gcc41/lib/libgdal.so.1 #9 0x0000000000000000 in ?? ()

Note on #4 that nIndex is -1093326368, when the code in CSVDeaccess calls CPLGetTLS(CTLS_CSVTABLEPTR), and CTLS_CSVTABLEPTR equals 4 in cpl_multiproc.h. So when I get the SEGV, something wrong must be happening before that gives the corruption. The crash itself occured the first time i had recompiled GDAL 1.10 to use the last versions of zlib, libpng and libjpeg, and only in debug. I recompiled with the same config and it seems to be working now. So the crash might be a mistake from my part. The ticket was more to point that i think it is useless to allocate memory in the TLS from code that is supposed to clear resources

[Even Rouault]

Your suggestion makes a theoretical sense, but in practice it would complicate the code more than needed for a very marginal use case (would need to be done for both Unix and Windows cases).

The segfault in malloc() itself above could indeed be linked to an error in another place than in GDAL