Mercurial > hg
comparison mercurial/parsers.c @ 25561:50a6c3c55db1 stable
parsers: do not cache RevlogError type (issue4451)
Index lookups raise RevlogError when the lookup fails. The previous
implementation was caching a reference to the RevlogError type in a
static variable. This assumed that the "mercurial.error" module was
only loaded once and there was only a single copy of it floating
around in memory. Unfortunately, in some situations - including
certain mod_wsgi configurations - this was not the case: the
"mercurial.error" module could be reloaded. It was possible for a
"RevlogError" reference from the first interpreter to be used by
a second interpreter. While the underlying thing was a
"mercurial.error.RevlogError," the object IDs were different, so
the Python code in revlog.py was failing to catch the exception! This
error has existed since the C index lookup code was implemented in
changeset e8d37b78acfb, which was first released in Mercurial 2.2 in
2012.
http://emptysqua.re/blog/python-c-extensions-and-mod-wsgi/#static-variables-are-shared
contains more details.
This patch removes the caching of the RevlogError type from the
function.
Since pretty much the entire function was refactored and the return
value of the function wasn't used, I changed the function signature
to not return anything.
For reasons unknown to me, we were calling PyErr_SetObject()
with the type of RevlogError and an instance of RevlogError. This
was equivalent to the Python code "raise RevlogError(RevlogError)".
This seemed wonky and completely unnecessary. The Python code only
cares about the type of the exception, not its contents. So I got
rid of this complexity.
This is my first Python C extension patch. Please give extra scrutiny
to it during review.
author | Gregory Szorc <gregory.szorc@gmail.com> |
---|---|
date | Fri, 12 Jun 2015 14:43:59 -0700 |
parents | b3142ea2a0d4 |
children | a69983942fb4 |
comparison
equal
deleted
inserted
replaced
25471:7298da81f5a9 | 25561:50a6c3c55db1 |
---|---|
1481 if (rev >= 0) | 1481 if (rev >= 0) |
1482 return rev; | 1482 return rev; |
1483 return -2; | 1483 return -2; |
1484 } | 1484 } |
1485 | 1485 |
1486 static PyObject *raise_revlog_error(void) | 1486 static void raise_revlog_error(void) |
1487 { | 1487 { |
1488 static PyObject *errclass; | 1488 PyObject *mod = NULL, *dict = NULL, *errclass = NULL; |
1489 PyObject *mod = NULL, *errobj; | 1489 |
1490 | 1490 mod = PyImport_ImportModule("mercurial.error"); |
1491 if (mod == NULL) { | |
1492 goto cleanup; | |
1493 } | |
1494 | |
1495 dict = PyModule_GetDict(mod); | |
1496 if (dict == NULL) { | |
1497 goto cleanup; | |
1498 } | |
1499 Py_INCREF(dict); | |
1500 | |
1501 errclass = PyDict_GetItemString(dict, "RevlogError"); | |
1491 if (errclass == NULL) { | 1502 if (errclass == NULL) { |
1492 PyObject *dict; | 1503 PyErr_SetString(PyExc_SystemError, |
1493 | 1504 "could not find RevlogError"); |
1494 mod = PyImport_ImportModule("mercurial.error"); | 1505 goto cleanup; |
1495 if (mod == NULL) | 1506 } |
1496 goto classfail; | 1507 |
1497 | 1508 /* value of exception is ignored by callers */ |
1498 dict = PyModule_GetDict(mod); | 1509 PyErr_SetString(errclass, "RevlogError"); |
1499 if (dict == NULL) | 1510 |
1500 goto classfail; | 1511 cleanup: |
1501 | 1512 Py_XDECREF(dict); |
1502 errclass = PyDict_GetItemString(dict, "RevlogError"); | |
1503 if (errclass == NULL) { | |
1504 PyErr_SetString(PyExc_SystemError, | |
1505 "could not find RevlogError"); | |
1506 goto classfail; | |
1507 } | |
1508 Py_INCREF(errclass); | |
1509 Py_DECREF(mod); | |
1510 } | |
1511 | |
1512 errobj = PyObject_CallFunction(errclass, NULL); | |
1513 if (errobj == NULL) | |
1514 return NULL; | |
1515 PyErr_SetObject(errclass, errobj); | |
1516 return errobj; | |
1517 | |
1518 classfail: | |
1519 Py_XDECREF(mod); | 1513 Py_XDECREF(mod); |
1520 return NULL; | |
1521 } | 1514 } |
1522 | 1515 |
1523 static PyObject *index_getitem(indexObject *self, PyObject *value) | 1516 static PyObject *index_getitem(indexObject *self, PyObject *value) |
1524 { | 1517 { |
1525 char *node; | 1518 char *node; |