Wed 31 May 2017 05:04:30 AM UTC, comment #8:
Tested patches in both branches with the demonstration code, no segfault.
|
Tue 30 May 2017 04:50:53 PM UTC, comment #7:
I think we can do the same thing in erase_scope: save pointer to table, remove element from the map, delete table. I pushed the following changesets to do that on stable and default:
http://hg.savannah.gnu.org/hgweb/octave/rev/25817ecc6123
http://hg.savannah.gnu.org/hgweb/octave/rev/61971e3f1946
Thanks.
|
Sun 28 May 2017 06:38:49 PM UTC, comment #6:
Yes, absolutely. Take this instead of my patch. But have you considered the part for symbtab.h (symbol_table::erase_scope), should the symbol_table pointer not be zeroed there in all_instances before deleting (a pointer copy)? (Although the demonstrated misbehaviour didn't depend on it.)
|
Sun 28 May 2017 05:07:33 PM UTC, comment #5:
Thanks for the report and patch. I think the attached version is a bit simpler and clearer.
(file #40804)
|
Wed 24 May 2017 10:05:42 AM UTC, comment #4:
The second half of the changeset is not necessary to fix the bug as it was currently triggered. But I thought a similar bug could occur if this change isn't made, too.
To explain the second half:
To your question: The destructor of the symbol_table (being empty, but automatically calling the destructors of the contained objects), is called by deleting with the copy 'inst' of 'p->second'. The '...->second' entries in 'all_instances' are zeroed before triggering the destructor call to avoid that the side effects of the destructor may try to trigger a destructor of a specific symbol table a second time (over 'all_instances').
This deleting with a pointer copy, after zeroing the original pointer, is the same as was done already before my patch in symtab.cc, symbol_table::cleanup(), to avoid that side effects call destructors of already destroyed symbol_tables. Here, in symbtab.h, symbol_table::erase_scope(), my change is only a precaution against the possibility that the side effects could try to destroy (over 'all_instances') the same symbol table as we currently already are destroying. Or can someone exclude that this happens? I'd rather be at the safe side.
|
Tue 23 May 2017 11:35:08 PM UTC, comment #3:
The changeset works. However, is the second half of the cset necessary?
I removed this part of the patch and it still continued to work. Also, it seems like we shouldn't be zeroing the pointer of the symbol_table since we want the symbol table destructor to be called if it exists. Thoughts on that?
|
Tue 23 May 2017 08:43:54 PM UTC, comment #2:
Confirmed. Even with the latest development branch code I get a segfault with the test code
|
Mon 22 May 2017 10:06:48 PM UTC, comment #1:
I have erroneously attached two versions of the changeset. Please take the later one.
Instead I had not attached the example code, which I do now.
(file #40762)
|
Mon 22 May 2017 10:00:38 PM UTC, original submission:
(tested with 4.2.1, but probably affects all current versions)
Under certain conditions Octave segfaults at exit due to a double free. Example code, involving anonymous functions and persistent variables, is attached in 'test-double-free.tar.gz', trigger the bug (hopefully) by going into the directory and calling 'test_gph ()' .
Debugging showed the following reason:
symbol_table::cleanup(void) iterates over symbol_table::all_instances and deletes symbol tables. As a side effect of such a delete, symbol_table::erase_scope(scope_id) erased a scope/symbol_table-pointer pair from all_instances. This erasing messed up the outer iteration over all_instances, so that after the next incrementing of the iterator it points to an invalid pair, causing an attempt to delete with the same symbol_table pointer which was deleted with by symbol_table::erase_scope(scope_id) .
A changeset with an explanatory comment will be attached as soon as I have the bug number. The patch is against the head of the stable branch, but has been tested against the 4.2.1 release tarball.
|