Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9cda81f

Browse files
committed
Be more careful about Python refcounts while creating exception objects.
PLy_generate_spi_exceptions neglected to do Py_INCREF on the new exceptionobjects, evidently supposing that PyModule_AddObject would do that --- butit doesn't. This left us in a situation where a Python garbage collectioncycle could result in deletion of exception object(s), causing servercrashes or wrong answers if the exception objects are used later in thesession.In addition, PLy_generate_spi_exceptions didn't bother to test fora null result from PyErr_NewException, which at best is inconsistentwith the code in PLy_add_exceptions. And PLy_add_exceptions, while itdid do Py_INCREF on the exceptions it makes, waited to do that tillafter some PyModule_AddObject calls, creating a similar risk forfailure if garbage collection happened within those calls.To fix, refactor to have just one piece of code that creates anexception object and adds it to the spiexceptions module, bumping therefcount first.Also, let's add an additional refcount to represent the pointer we'regoing to store in a C global variable or hash table. This should onlymatter if the user does something weird like delete the spiexceptionsPython module, but lack of paranoia has caused us enough problems inPL/Python already.The fact that PyModule_AddObject doesn't do a Py_INCREF of its ownexplains the need for the Py_INCREF added in commit4c966d9, so wecan improve the comment about that; also, this means we really wantto do that before not after the PyModule_AddObject call.The missing Py_INCREF in PLy_generate_spi_exceptions was reported anddiagnosed by Rafa de la Torre; the other fixes by me. Back-patchto all supported branches.Discussion:https://postgr.es/m/CA+Fz15kR1OXZv43mDrJb3XY+1MuQYWhx5kx3ea6BRKQp6ezGkg@mail.gmail.com
1 parenta73491e commit9cda81f

File tree

1 file changed

+50
-29
lines changed

1 file changed

+50
-29
lines changed

‎src/pl/plpython/plpy_plpymodule.c

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ HTAB *PLy_spi_exceptions = NULL;
2525

2626

2727
staticvoidPLy_add_exceptions(PyObject*plpy);
28+
staticPyObject*PLy_create_exception(char*name,
29+
PyObject*base,PyObject*dict,
30+
constchar*modname,PyObject*mod);
2831
staticvoidPLy_generate_spi_exceptions(PyObject*mod,PyObject*base);
2932

3033
/* module functions */
@@ -192,46 +195,62 @@ PLy_add_exceptions(PyObject *plpy)
192195
#else
193196
excmod=PyModule_Create(&PLy_exc_module);
194197
#endif
195-
if (PyModule_AddObject(plpy,"spiexceptions",excmod)<0)
196-
PLy_elog(ERROR,"could notadd the spiexceptions module");
198+
if (excmod==NULL)
199+
PLy_elog(ERROR,"could notcreate the spiexceptions module");
197200

198201
/*
199-
* XXX it appears that in some circumstances the reference count of the
200-
* spiexceptions module drops to zero causing a Python assert failure when
201-
* the garbage collector visits the module. This has been observed on the
202-
* buildfarm. To fix this, add an additional ref for the module here.
203-
*
204-
* This shouldn't cause a memory leak - we don't want this garbage
205-
* collected, and this function shouldn't be called more than once per
206-
* backend.
202+
* PyModule_AddObject does not add a refcount to the object, for some odd
203+
* reason; we must do that.
207204
*/
208205
Py_INCREF(excmod);
206+
if (PyModule_AddObject(plpy,"spiexceptions",excmod)<0)
207+
PLy_elog(ERROR,"could not add the spiexceptions module");
209208

210-
PLy_exc_error=PyErr_NewException("plpy.Error",NULL,NULL);
211-
PLy_exc_fatal=PyErr_NewException("plpy.Fatal",NULL,NULL);
212-
PLy_exc_spi_error=PyErr_NewException("plpy.SPIError",NULL,NULL);
213-
214-
if (PLy_exc_error==NULL||
215-
PLy_exc_fatal==NULL||
216-
PLy_exc_spi_error==NULL)
217-
PLy_elog(ERROR,"could not create the base SPI exceptions");
218-
219-
Py_INCREF(PLy_exc_error);
220-
PyModule_AddObject(plpy,"Error",PLy_exc_error);
221-
Py_INCREF(PLy_exc_fatal);
222-
PyModule_AddObject(plpy,"Fatal",PLy_exc_fatal);
223-
Py_INCREF(PLy_exc_spi_error);
224-
PyModule_AddObject(plpy,"SPIError",PLy_exc_spi_error);
209+
PLy_exc_error=PLy_create_exception("plpy.Error",NULL,NULL,
210+
"Error",plpy);
211+
PLy_exc_fatal=PLy_create_exception("plpy.Fatal",NULL,NULL,
212+
"Fatal",plpy);
213+
PLy_exc_spi_error=PLy_create_exception("plpy.SPIError",NULL,NULL,
214+
"SPIError",plpy);
225215

226216
memset(&hash_ctl,0,sizeof(hash_ctl));
227217
hash_ctl.keysize=sizeof(int);
228218
hash_ctl.entrysize=sizeof(PLyExceptionEntry);
229-
PLy_spi_exceptions=hash_create("SPI exceptions",256,
219+
PLy_spi_exceptions=hash_create("PL/PythonSPI exceptions",256,
230220
&hash_ctl,HASH_ELEM |HASH_BLOBS);
231221

232222
PLy_generate_spi_exceptions(excmod,PLy_exc_spi_error);
233223
}
234224

225+
/*
226+
* Create an exception object and add it to the module
227+
*/
228+
staticPyObject*
229+
PLy_create_exception(char*name,PyObject*base,PyObject*dict,
230+
constchar*modname,PyObject*mod)
231+
{
232+
PyObject*exc;
233+
234+
exc=PyErr_NewException(name,base,dict);
235+
if (exc==NULL)
236+
PLy_elog(ERROR,"could not create exception \"%s\"",name);
237+
238+
/*
239+
* PyModule_AddObject does not add a refcount to the object, for some odd
240+
* reason; we must do that.
241+
*/
242+
Py_INCREF(exc);
243+
PyModule_AddObject(mod,modname,exc);
244+
245+
/*
246+
* The caller will also store a pointer to the exception object in some
247+
* permanent variable, so add another ref to account for that. This is
248+
* probably excessively paranoid, but let's be sure.
249+
*/
250+
Py_INCREF(exc);
251+
returnexc;
252+
}
253+
235254
/*
236255
* Add all the autogenerated exceptions as subclasses of SPIError
237256
*/
@@ -257,12 +276,14 @@ PLy_generate_spi_exceptions(PyObject *mod, PyObject *base)
257276

258277
PyDict_SetItemString(dict,"sqlstate",sqlstate);
259278
Py_DECREF(sqlstate);
260-
exc=PyErr_NewException(exception_map[i].name,base,dict);
261-
PyModule_AddObject(mod,exception_map[i].classname,exc);
279+
280+
exc=PLy_create_exception(exception_map[i].name,base,dict,
281+
exception_map[i].classname,mod);
282+
262283
entry=hash_search(PLy_spi_exceptions,&exception_map[i].sqlstate,
263284
HASH_ENTER,&found);
264-
entry->exc=exc;
265285
Assert(!found);
286+
entry->exc=exc;
266287
}
267288
}
268289

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp