On Mon, 26 Apr 2004, Carl Hetherington wrote:

On Mon, 26 Apr 2004, bulia byak wrote:

...

...

which means that the SP_DT_SELECTION macro will give an assertion failure if it doesn't return a non-NULL selection. I've then removed defensive checks from callers of SP_DT_SELECTION. Everything seems ok so far.

Here's a crash that seems to be caused by selection==NULL:

http://sourceforge.net/tracker/index.php?func=detail&aid=942149&grou...

Thanks Bulia, having a look now.

OK, this crash is caused by the following sequence:

sp_dtw_desktop_shutdown() (signal handler) sp_desktop_prepare_shutdown() desktop->selection = NULL inkscape_remove_desktop() which emits a SET_SELECTION to NULL.

this is then picked up by text-edit.cpp. This eventually calls itemList() for SP_DT_SELECTION(SP_ACTIVE_DESKTOP). By this time, the active desktop's selection is NULL.

There seem to be lots of ways of fixing this, and I'm really unsure as to which is best:

1. Don't call sp_desktop_prepare_shutdown(). I can't see what it's trying to achieve, given that everything it does will be performed by sp_desktop_dispose(). Also, sp_desktop_dispose() does the inkscape_remove_desktop before the selection = NULL, meaning that the crash will not occur.

2. Rearrange sp_desktop_prepare_shutdown() to do things in the same order as sp_desktop_dispose().

3. Make text-edit.cpp look at the new SPSelection* that is passed in by the SET_SELECTION event, rather than using SP_ACTIVE_DESKTOP. Then it could check for NULLness.

4. Change inkscape_remove_desktop() to call selection->clear() rather than SET_SELECTION to NULL. Then leave sp_desktop_dispose() to actually unref the SPSelection.

Any thoughts?

Thanks

Carl

[snip]

I would personally vote for 3, 4 _and_ 1. There will probably be some corner cases to work out, but it should significantly improve the design.

Thanks for taking a look, mental.

I've made these changes in my local tree, and I'll test them over the next day or two before committing.

Cheers

Carl