Fix T92655: spreadsheet_duplicate Split Exception
ClosedPublic
Actions

Authored by Harley Acheson (harley) on Oct 30 2021, 9:57 PM.

Details

Reviewers

Julian Eisel (Severin)
Jacques Lucke (JacquesLucke)

Commits

rBadc540cf7ccf: Fix T92655: spreadsheet_duplicate Split Exception
rB69e504225804: Fix T92655: spreadsheet_duplicate Split Exception

Summary

Check SpaceSpreadsheet's runtime is not null when trying to duplicate
the data when doing an area split.

Each ScrArea keeps a list containing the runtime data used by the editors used previously. This way you can change from one editor type to another and back and you see your previous options and selections.

For the example blend provided in T92655 the 3DView's previous editors included Spreadsheet, but for some reason its runtime member is a nullptr, which causes a crash in the constructor. This patch just ensures we call the correct constructor in this weird case.

It is certainly possible that there is a prettier way to do this, or that this should be checking instead in the constructor, or that the reason for this state should be fixed rather than tested for,

Diff Detail

Repository: rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Oct 30 2021, 9:57 PM

Harley Acheson (harley) created this revision.

Jesse Yurkovich (deadpin) mentioned this in T92667: Opening a certain file and splitting the main editor results in a crash.Oct 31 2021, 5:29 AM

Think the fix makes sense.
I wonder if we should actually call the init method for all spaces in an area when a file is loaded. But that can be a separate discussion.

This revision is now accepted and ready to land.Oct 31 2021, 12:32 PM

In itself this looks fine to me.

I'm wondering though, any reason not to use OBJECT_GUARDED_NEW()/OBJECT_GUARDED_DELETE()?

@Julian Eisel (Severin) - …any reason not to use OBJECT_GUARDED_NEW()/OBJECT_GUARDED_DELETE()?

I’m assuming this question was for @Jacques Lucke (JacquesLucke) as I don’t have a clue. LOL

@Harley Acheson (harley) Please just commit the patch now :)

@Julian Eisel (Severin) It could probably just as well use OBJECT_GUARDED_NEW. Can change that separately. And maybe we should consider improving the API a bit to make it less annoying, e.g. by introducting T* MEM_new<T>(Args...) and void MEM_delete(T *), but that's a different topic.

In D13047#344450, @Jacques Lucke (JacquesLucke) wrote:

@Julian Eisel (Severin) It could probably just as well use OBJECT_GUARDED_NEW. Can change that separately. And maybe we should consider improving the API a bit to make it less annoying, e.g. by introducting T* MEM_new<T>(Args...) and void MEM_delete(T *), but that's a different topic.

+1 on that. At least no bare new/delete that's not even using the guarded allocator ;)

Closed by commit rB69e504225804: Fix T92655: spreadsheet_duplicate Split Exception (authored by Harley Acheson (harley)). · Explain WhyNov 1 2021, 4:49 PM

This revision was automatically updated to reflect the committed changes.

Harley Acheson (harley) added a commit: rB69e504225804: Fix T92655: spreadsheet_duplicate Split Exception.

Harley Acheson (harley) added a commit: rBadc540cf7ccf: Fix T92655: spreadsheet_duplicate Split Exception.Nov 1 2021, 4:56 PM