[gdal-dev] Potential dangerous use of CPLSetErrorHandler()
Roger Bivand
Roger.Bivand at nhh.no
Fri Feb 18 03:58:11 EST 2011
On Fri, 18 Feb 2011, Even Rouault wrote:
> Le vendredi 18 février 2011 01:02:30, Even Rouault a écrit :
>>
>> 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()
Even,
This analysis holds in a shared object/DLL world, right? For our static
builds of rgdal.dll for Windows 32/64-bit, there would be no interaction?
So it's a case of dynamic linking having rather tricky pitfalls?
In GRASS, the modules linked to GDAL/OGR are short-lived, and over the
years, nobody has reported any issues (though that isn't hard evidence),
but QGIS and R are running together over longer periods.
I'm adding Carson Farmer to the CCs in case he is not subscribed here - he
has some nice R/QGIS interfaces, and may be able to help in setting up a
test rig.
Note that R runs single threaded (plus a GUI thread if there is a GUI),
now because it is useful to run separate parallel R threads across
multicore processors, and the R engine is small.
I feel that we need a test rig to be able to provoke the problem
predictably, so that we know when it is settled, for known versions of
GDAL/OGR, R, and rgdal, and versions of QGIS if we cannot provoke the
problem without it.
From the initial report from Bob, it isn't clear (for me) how R/rgdal came
into the picture? Is R with rgdal loaded running but separate from QGIS,
or is R being called (even through system or via python) from QGIS?
Bob says on IRC that the problem occurs when the MCE Lite plugin is loaded
in QGIS, but not otherwise - the plugin does not use R or rgdal. Could Bob
confirm that this is the case, or does the problem occur independently of
MCE Lite being loaded? Do we know which QGIS plugin is loading R and
rgdal, and how?
Since that part of rgdal dates back at least 8 years, it may be worth
revisiting, but I'd like to have a fuller analysis and test rig before
modifying hundreds of functions calling GDAL and OGR in rgdal.
Roger
>>
>> For example :
>>
>> SEXP
>> RGDAL_GetDescription(SEXP sxpObj) {
>>
>> void *pGDALObj = getGDALObjPtr(sxpObj);
>>
>> CPLPushErrorHandler(__errorHandler);
>>
>> const char *desc = ((GDALMajorObject *)pGDALObj)->GetDescription();
>>
>> CPLPopErrorHandler();
>>
>> return(mkString_safe(desc));
>>
>> }
>>
>
> Actually, I'm just thinking that when an error actually triggers, the
> CPLPopErrorHandler() will not be called due to the longjmp() done in the error
> handler... So you probably needs to call CPLPopErrorHandler() in the error
> handler before it to call error() (a quick review of port/cpl_error.cpp leads
> me to think this is OK to do so, but this needs certainly some testing)
>
> Or other possibility, it might be dangerous that the error handler interrupts
> the GDAL code flow with the longjmp(). It could cause memory leaks or leaves
> GDAL in an unstable state (unreleased locks, or whatever). What would be safer
> would be that the error handler just stores the error code and/or message, and
> just looks at it after the return from the GDAL code.
>
> Something like this :
>
> static CPLErr saved_eErrClass = CE_None;
> static saved_err_no = 0;
> static char saved_error_msg[2048];
>
> static void
> __errorHandler(CPLErr eErrClass, int err_no, const char *msg) {
> saved_eErrClass = eErrClass;
> saved_err_no = err_no;
> /* a mutex could be usefull here to avoid a race condition if 2 threads
> trigger the error handler at the same time */
> strncpy(saved_error_msg, msg, sizeof(saved_error_msg));
> saved_error_msg[sizeof(saved_error_msg)-1] = 0;
> }
>
> void installErrorHandler()
> {
> CPLPushErrorHandler(__errorHandler);
> saved_err_no = 0;
> }
>
> void uninstallErrorHandlerAndTriggerRError()
> {
> CPLPopErrorHandler();
> if (saved_err_no == CE_Warning) {
>
> warning("\n\tNon-fatal GDAL Error %d: %s\n", saved_err_no,
> saved_error_msg);
>
> } else if (saved_err_no == CE_Failure) {
>
> error("\n\tGDAL Error %d: %s\n", saved_err_no, saved_error_msg);
>
> }
> }
>
> SEXP
> RGDAL_GetDescription(SEXP sxpObj) {
>
> void *pGDALObj = getGDALObjPtr(sxpObj);
>
> installErrorHandler();
>
> const char *desc = ((GDALMajorObject *)pGDALObj)->GetDescription();
>
> uninstallErrorHandlerAndTriggerRError();
>
> return(mkString_safe(desc));
>
> }
>
> Or you could also take the approach of
> gdal/swig/include/python/python_exceptions.i where the installed error handler
> just forwards CE_Fatal, CE_Warning and CE_Debug to the previous error handler
> (or the default one if there's none). After the GDAL call, it just looks at
> CPLGetLastErrorType() and if it is CE_Failure, it triggers a Python exception
> with the error messages fetched with CPLGetLastErrorMsg().
>
--
Roger Bivand
Economic Geography Section, Department of Economics, Norwegian School of
Economics and Business Administration, Helleveien 30, N-5045 Bergen,
Norway. voice: +47 55 95 93 55; fax +47 55 95 95 43
e-mail: Roger.Bivand at nhh.no
More information about the gdal-dev
mailing list