Fixes for gevent after running with valgrind #1474
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This seemed to be caused by something decrementing reference counts a bit too zealously, but was a bit hard to track down. At one point I tried running with a two gevent cores and this made the error more obvious. I'd been running with a debug build of Python as well as uWSGI under Valgrind and I was getting various errors when using the gevent plugin, for example: Fatal Python error: plugins/python/python_plugin.c:1099 object at 0xcb15380 has negative ref count -1 ==22053== ==22053== Process terminating with default action of signal 6 (SIGABRT): dumping core ==22053== at 0x70E204F: raise (in /usr/lib/libc-2.24.so) ==22053== by 0x70E3479: abort (in /usr/lib/libc-2.24.so) ==22053== by 0x5C86ED: Py_FatalError (pythonrun.c:1700) ==22053== by 0x51F187: _Py_NegativeRefcount (object.c:200) ==22053== by 0x49A625: uwsgi_request_wsgi (wsgi_handlers.c:429) ==22053== by 0x4A8C64: py_uwsgi_gevent_request (gevent.c:317) ==22053== by 0x51DC36: PyCFunction_Call (methodobject.c:81) ==22053== by 0x598EEA: ext_do_call (ceval.c:4663) ==22053== by 0x592858: PyEval_EvalFrameEx (ceval.c:3028) ==22053== by 0x5951EC: PyEval_EvalCodeEx (ceval.c:3584) ==22053== by 0x4FB849: function_call (funcobject.c:523) ==22053== by 0x4CD162: PyObject_Call (abstract.c:2547) and: ==15873== Invalid read of size 8 ==15873== at 0x533F6A: _PyString_Eq (stringobject.c:1258) ==15873== by 0x5144A1: lookdict_string (dictobject.c:435) ==15873== by 0x515497: PyDict_GetItem (dictobject.c:743) ==15873== by 0x5221D1: _PyObject_GenericGetAttrWithDict (object.c:1428) ==15873== by 0x5224AB: PyObject_GenericGetAttr (object.c:1461) ==15873== by 0x521911: PyObject_GetAttr (object.c:1196) ==15873== by 0x521637: PyObject_GetAttrString (object.c:1138) ==15873== by 0x4A86BA: uwsgi_gevent_current_wsgi_req (gevent.c:146) ==15873== by 0x49EF86: py_uwsgi_websocket_handshake (uwsgi_pymodule.c:1082) ==15873== by 0x51DC36: PyCFunction_Call (methodobject.c:81) ==15873== by 0x597A1E: call_function (ceval.c:4352) ==15873== by 0x5924D2: PyEval_EvalFrameEx (ceval.c:2989) ==15873== Address 0xc2cd4f0 is 32 bytes inside a block of size 67 free'd ==15873== at 0x4C2BD3A: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==15873== by 0x524A64: PyObject_Free (obmalloc.c:1325) ==15873== by 0x52F6EF: string_dealloc (stringobject.c:597) ==15873== by 0x524167: _Py_Dealloc (object.c:2262) ==15873== by 0x515FB4: dict_dealloc (dictobject.c:1039) ==15873== by 0x524167: _Py_Dealloc (object.c:2262) ==15873== by 0x514A1F: insertdict_by_entry (dictobject.c:519) ==15873== by 0x514CC5: insertdict (dictobject.c:556) ==15873== by 0x51562B: dict_set_item_by_hash_or_entry (dictobject.c:795) ==15873== by 0x5157B1: PyDict_SetItem (dictobject.c:848) ==15873== by 0x519F76: PyDict_SetItemString (dictobject.c:2469) ==15873== by 0x49B9B7: uwsgi_request_subhandler_wsgi (wsgi_subhandler.c:223) ==15873== Block was alloc'd at ==15873== at 0x4C2AB8D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==15873== by 0x52E362: PyString_FromString (stringobject.c:143) ==15873== by 0x53E06E: PyString_InternFromString (stringobject.c:4797) ==15873== by 0x5217A4: PyObject_SetAttrString (object.c:1163) ==15873== by 0x4A8C15: py_uwsgi_gevent_request (gevent.c:278) ==15873== by 0x51DC36: PyCFunction_Call (methodobject.c:81) ==15873== by 0x598EEA: ext_do_call (ceval.c:4663) ==15873== by 0x592858: PyEval_EvalFrameEx (ceval.c:3028) ==15873== by 0x5951EC: PyEval_EvalCodeEx (ceval.c:3584) ==15873== by 0x4FB849: function_call (funcobject.c:523) ==15873== by 0x4CD162: PyObject_Call (abstract.c:2547) ==15873== by 0x4E4641: instancemethod_call (classobject.c:2602) (followed by more apparent use after free/reference leaks)
unbit
reviewed
Feb 23, 2017
| @@ -1092,7 +1092,6 @@ void *uwsgi_python_create_env_holy(struct wsgi_request *wsgi_req, struct uwsgi_a | |||
| } | |||
|
|
|||
| void uwsgi_python_destroy_env_holy(struct wsgi_request *wsgi_req) { | |||
| Py_DECREF((PyObject *)wsgi_req->async_environ); | |||
There was a problem hiding this comment.
Not sure about this, in the holy mode we explicitely destroy the environment
There was a problem hiding this comment.
yup, wasn't sure about this. but there seem to be two DECREFs straight after each other
I don't know the code well enough to know where ownership belongs. maybe if I put a small test together than can be run under different threading/async modes?
|
Thanks, see the comments about the python_plugin.c part |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My attempt at fixing reference errors when running with
gevent. I was experiencing strange crashes while running through the article on uWSGI and SSE's.The "real" change is in 3039e35 and might not be correct, but the two calls to
Py_DECREFlook suspect to me. I've arbitrarily removed one and a series of strange crashes stopped occuring for me. Threading and gevent seems to be unsupported at the moment, hence saying there are two calls. I don't know the code well enough to do any better with this at the moment!Let me know if you think this is about right!