[gdal-dev] Potential dangerous use of CPLSetErrorHandler()

[Even Rouault]

Hi Roger,

not sure if you are the appropriate recipient for this email, but as your name 
appeared on http://cran.r-project.org/web/packages/rgdal/index.html I try my 
chance ;-).

(CC'ing gdal-dev too since other users of GDAL might be interested by the 
issue related with CPLSetErrorHandler() in other contexts)

I had a discussion with Bob on the GDAL IRC that you can follow here :

http://logs.qgis.org/gdal/%23gdal.2011-02-16.log and
http://logs.qgis.org/gdal/%23gdal.2011-02-17.log

Here's my analysis (based on https://r-forge.r-
project.org/scm/viewvc.php/trunk/src/gdal-bindings.cpp?view=markup&root=rgdal 
) of what leads to the crash he noticed (of course I can be wrong since my 
knowledge of R is null, so this is mostly based on guess) :
1) QGIS loads a plugin that loads RGDAL
2) When RGDAL is initialized, it installs a global error handler 
__errorHandler with CPLSetErrorHandler()
3) Some functions of QGIS unrelated to RGDAL causes a CPLError() to be emitted 
by GDAL/OGR
4) The global RGDAL error handler is triggered, causes the error() method to 
be called which apparently causes a longjmp() to the R interpreter
5) At that point QGIS crashes as the longjmp() is inappropriate because R 
isn't ready for being run at that point

So my suggestion would be not to use a global error handler set when RGDAL is 
initialized, but rather for each binding of the GDAL API, install a local 
error handler with CPLPushErrorHandler(__errorHandler) and uninstall it before 
returning to R with CPLPopErrorHandler()

For example :

SEXP
RGDAL_GetDescription(SEXP sxpObj) {

  void *pGDALObj = getGDALObjPtr(sxpObj);

  CPLPushErrorHandler(__errorHandler);

  const char *desc = ((GDALMajorObject *)pGDALObj)->GetDescription();

  CPLPopErrorHandler();

  return(mkString_safe(desc));

}

This is going to be quite painfull since you have to do this for each GDAL 
method you bind, and be careful to correctly pair CPLPushErrorHandler() / 
CPLPopErrorHandler() (the later being the easiest to forget in unsuual code 
paths, such as error code paths).

Best regards,

Even