New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
errors on _bsddb creation and dealloc #48135
Comments
I found two differents bugs using Fusil the fuzzer. (1) On db_env_create() error in newDBEnvObject(), self->db_env is not (2) DBEnv_dealloc() may raise an exception (DBEnv_close_internal() Example of (2): $ python
>>> import gc, _bsddb; env=_bsddb.DBEnv(3); del env
>>> gc.collect(); gc.collect()
Exception bsddb.db.DBNoServerError: (-30992, 'DB_NOSERVER: Fatal
error, no RPC server -- No Berkeley DB RPC server environment')
in 'garbage collection' ignored
Fatal Python error: unexpected exception during garbage collection |
About the bug (1): it also occurs in DBEnv_dealloc() but Backtrace using gdb: $ gdb ./python
...
>>> import _bsddb; _bsddb.DBenv(92)
...
Program received signal SIGSEGV, Segmentation fault. 0xb7cc8f5e in DBEnv_close_internal (self=0x83385f0, flags=0) There db_env value is not set. |
Good catch. I've patched my private branch. Following 2.6 patches, if I was wondering if we must protect other objects from raising exceptions What do you think?. (I have broken my left wrist ten days ago. If you can write a testcase |
Here is a test to reproduce the crash. I don't know db_env_create() About other objects dealloc method... DBEnv_dealloc()
`-> DBEnv_close_internal(): RETURN_IF_ERR()
DB_dealloc()
`-> DB_close_internal(): RETURN_IF_ERR()
DBC_dealloc()
`-> DBC_close_internal(): RETURN_IF_ERR()
DBSequence_dealloc()
`-> DBSequence_close_internal(): RETURN_IF_ERR()
DBTxn_dealloc()
`-> DBTxn_abort_discard_internal(): RETURN_IF_ERR() DBLock_dealloc() looks ok. |
I thought first that the problem was during the execution of >>> import gc, _bsddb; env=_bsddb.DBEnv(3); del env
XXX undetected error
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
bsddb.db.DBNoServerError: (-30992, 'DB_NOSERVER: Fatal error, no RPC
server -- No Berkeley DB RPC server environment') gc.collect() is just a rude way to display this "XXX undetected error". Now, to the _bsddb module: in general, the following pattern is wrong:
dummy = someFunction();
Py_XDECREF(dummy);
because it does nothing about an eventual exception set. If the
exception can be discarded, PyErr_Clear() must be called. I think there is an invariant to keep in each function: return NULL if |
I stopped to fuzz Python using --pydebug because a critical design error in
Well, we have to choices: don't raise an error, or clear the exception if an
Where? dealloc() callback prototype is "void tp_dealloc(...)": no result! It's |
So, the right thing to do seems to drop the "check_error" flag and just This must be done in all dealloc code: DBEnv, DB, DBSequence and DBCursor. Do you agree?. |
Not only in these functions, but anywhere "dummy=" is followed by a
Py_XDECREF(dummy);
And code like this must also be changed (in DB_open):
if (makeDBError(err)) {
PyObject *dummy;
dummy=DB_close_internal(self,0);
Py_XDECREF(dummy);
return NULL;
}
if DB_close_internal() raises an exception it will replace the original
error set by makeDBError(). I'm not sure this is desirable. |
I won't comment on this, but to get the extra checks you could arrange
Exactly. You cannot return NULL (or -1 for other kind of functions), so |
Please, clarify this. I don't see your point.
I agree this could be an issue that need to be studied. Please, post a I post a patch for the bugs originally indicated in this bug report, an *Please Review*. I will update python 2.6 svn when the review is done. |
If dummy is NULL, a pending exception has been set (with PyErr_SetString |
Very good point, Amaury. I would open a new bug to track this (probably for 2.6.1 time). Can you?. In the meanwhile, I need review for the inmediate patch, since the crash |
Patch comments:
|
Nick:
I can split the patch, if demanded, nevertheless.
""" Similarly in "DBSequence.remove()". I'm actually thinking about splitting "*_close_internal()" functions in
Other "dummy=DB_close_internal(*) + Py_XDECREF" uses need be improved,
If you don't agree, please tell to me. Would be a few "fprintf()" enough?. Thanks for your time, Nick. |
All of those explanations sound fair to me - with those questions |
Thanks. Committed as r66568. |
Ok, thanks. My initial bug is closed and my fuzzer is unable to find |
Would be nice to use that fuzzer myself. Details, please :). |
It's not the best place to explain it, so I will try explain shortly:
The best way is to use the trunk version (without installation)! You have to run the fuzzer as root to be able to run child processes as the |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: