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;