From b0abeea43f6e51c452af68d8ec9bf3d0f701d772 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Sun, 12 Jul 2009 21:22:58 +0200 Subject: python/acquire.cc: Fix segmentation faults, introduce PyAcquireObject. Make AcquireItem objects raise ValueError instead of segfaulting when the Acquire() object is shut down or the main object (e.g. AcquireFile) is deallocated. This is implemented by using a vector of the AcquireItem objects, and setting AcquireItem->Object = NULL, when the memory 'Object' previously pointed to is going to be deleted. --- python/acquire.cc | 96 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 70 insertions(+), 26 deletions(-) (limited to 'python') diff --git a/python/acquire.cc b/python/acquire.cc index 6c6c2ea4..054956ea 100644 --- a/python/acquire.cc +++ b/python/acquire.cc @@ -12,15 +12,27 @@ #include +typedef CppPyObject PyAcquireItemObject; + +// Keep a vector to PyAcquireItemObject pointers, so we can set the Object +// pointers to NULL when deallocating the main object (mostly AcquireFile). +struct PyAcquireObject : public CppPyObject { + vector items; +}; + +inline pkgAcquire::Item *PyAcquireItem_ToCpp(PyObject *self) { + pkgAcquire::Item *itm = GetCpp(self); + if (itm == 0) + PyErr_SetString(PyExc_ValueError, "Acquire() has been shut down or " + "the AcquireFile() object has been deallocated."); + return itm; +} + #define MkGet(PyFunc,Ret) static PyObject *PyFunc(PyObject *Self,void*) \ { \ - pkgAcquire::Item *Itm; \ - if (PyObject_TypeCheck(Self, &PkgAcquireFileType)) { \ - Itm = GetCpp(Self); \ - } else { \ - pkgAcquire::ItemIterator &I = GetCpp(Self); \ - Itm = *I; \ - } \ + pkgAcquire::Item *Itm = PyAcquireItem_ToCpp(Self); \ + if (Itm == 0) \ + return 0; \ return Ret; \ } @@ -30,7 +42,7 @@ MkGet(AcquireItemGetDescURI,Safe_FromString(Itm->DescURI().c_str())); MkGet(AcquireItemGetDestFile,Safe_FromString(Itm->DestFile.c_str())); MkGet(AcquireItemGetErrorText,Safe_FromString(Itm->ErrorText.c_str())); MkGet(AcquireItemGetFileSize,Py_BuildValue("i",Itm->FileSize)); -MkGet(AcquireItemGetID,Py_BuildValue("i",Itm->ID)); +MkGet(AcquireItemGetID,Py_BuildValue("i",Itm)); MkGet(AcquireItemGetIsTrusted,Py_BuildValue("i",Itm->IsTrusted())); MkGet(AcquireItemGetLocal,Py_BuildValue("i",Itm->Local)); MkGet(AcquireItemGetStatus,Py_BuildValue("i",Itm->Status)); @@ -82,14 +94,9 @@ static PyGetSetDef AcquireItemGetSet[] = { static PyObject *AcquireItemRepr(PyObject *Self) { - pkgAcquire::Item *Itm; - if (PyObject_TypeCheck(Self, &PkgAcquireFileType)) { - Itm = GetCpp(Self); - } else { - pkgAcquire::ItemIterator &I = GetCpp(Self); - Itm = *I; - } - + pkgAcquire::Item *Itm = PyAcquireItem_ToCpp(Self); + if (Itm == 0) + return 0; char S[300]; snprintf(S,sizeof(S),"<%s object: " "Status: %i Complete: %i Local: %i IsTrusted: %i " @@ -102,15 +109,45 @@ static PyObject *AcquireItemRepr(PyObject *Self) return PyString_FromString(S); } +static void AcquireItemDealloc(PyObject *self) { + pkgAcquire::Item *file = GetCpp(self); + PyAcquireObject *owner = (PyAcquireObject *)GetOwner(self); + vector &items = owner->items; + bool DeletePtr = !((CppPyObject *)self)->NoDelete; + + // Cleanup the other objects as well... + for (vector::iterator I = items.begin(); + I != items.end(); I++) { + // If it is the current object, remove it from the vector. + if ((*I) == self) { + items.erase(I); + I--; // erase() moved it one forward, move back + } + // If we delete the object below, set all other pointers to it to NULL, + // and remove them from the vector. + else if (DeletePtr && (*I)->Object == file) { + if ((*I) != self) + (*I)->Object = NULL; + items.erase(I); + I--; // erase() moved it one forward, move back + } + } +#ifdef ALLOC_DEBUG + std::cerr << "ITEMS: " << items.size() << "\n"; +#endif + CppOwnedDeallocPtr(self); +} + + PyTypeObject AcquireItemType = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "apt_pkg.AcquireItem", // tp_name - sizeof(CppOwnedPyObject), // tp_basicsize + sizeof(CppOwnedPyObject), // tp_basicsize 0, // tp_itemsize // Methods - CppOwnedDealloc, // tp_dealloc + AcquireItemDealloc, // tp_dealloc 0, // tp_print 0, // tp_getattr 0, // tp_setattr @@ -139,6 +176,7 @@ PyTypeObject AcquireItemType = }; + static PyObject *PkgAcquireRun(PyObject *Self,PyObject *Args) { pkgAcquire *fetcher = GetCpp(Self); @@ -161,6 +199,11 @@ static PyObject *PkgAcquireShutdown(PyObject *Self,PyObject *Args) fetcher->Shutdown(); + vector items = ((PyAcquireObject *)Self)->items; + for (vector::iterator I = items.begin(); + I != items.end(); I++) + (*I)->Object = NULL; + Py_INCREF(Py_None); return HandleErrors(Py_None); } @@ -194,12 +237,12 @@ static PyObject *PkgAcquireGetItems(PyObject *Self,void*) for (pkgAcquire::ItemIterator I = fetcher->ItemsBegin(); I != fetcher->ItemsEnd(); I++) { - PyObject *Obj; - Obj = CppOwnedPyObject_NEW(Self, - &AcquireItemType,I); + PyAcquireItemObject *Obj; + Obj = CppOwnedPyObject_NEW(Self,&AcquireItemType,*I); + Obj->NoDelete = true; PyList_Append(List,Obj); + ((PyAcquireObject *)Self)->items.push_back(Obj); Py_DECREF(Obj); - } return List; } @@ -266,7 +309,7 @@ PyTypeObject PkgAcquireType = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "apt_pkg.Acquire", // tp_name - sizeof(CppPyObject), // tp_basicsize + sizeof(PyAcquireObject), // tp_basicsize 0, // tp_itemsize // Methods CppDeallocPtr, // tp_dealloc @@ -341,6 +384,8 @@ static PyObject *PkgAcquireFileNew(PyTypeObject *type, PyObject *Args, PyObject destFile); // short-desc CppOwnedPyObject *AcqFileObj = CppOwnedPyObject_NEW(pyfetcher, type); AcqFileObj->Object = af; + ((PyAcquireObject *)pyfetcher)->items.push_back((PyAcquireItemObject *)AcqFileObj); + return AcqFileObj; } @@ -359,9 +404,7 @@ PyTypeObject PkgAcquireFileType = sizeof(CppOwnedPyObject),// tp_basicsize 0, // tp_itemsize // Methods - // Not ..Ptr, because this would cause the item to be removed from the - // fetcher, which would be incompatible to previous behaviour. - CppOwnedDealloc, // tp_dealloc + AcquireItemDealloc, // tp_dealloc 0, // tp_print 0, // tp_getattr 0, // tp_setattr @@ -431,6 +474,7 @@ PyObject *GetPkgAcqFile(PyObject *Self, PyObject *Args, PyObject * kwds) destFile); // short-desc CppPyObject *AcqFileObj = CppPyObject_NEW(&PkgAcquireFileType); AcqFileObj->Object = af; + AcqFileObj->NoDelete = true; return AcqFileObj; } -- cgit v1.2.3