Skip to content
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

Deleting a non-empty recipe folder causes a crash #622

Closed
TechnicolorNarwhal opened this issue Oct 19, 2021 · 5 comments
Closed

Deleting a non-empty recipe folder causes a crash #622

TechnicolorNarwhal opened this issue Oct 19, 2021 · 5 comments
Assignees
Labels

Comments

@TechnicolorNarwhal
Copy link

If a recipe folder is deleted and there is a recipe in it, Brewtarget crashes with "Segmentation fault (core dumped)". However, the folder can be deleted if all of the recipes are deleted first.

@matty0ung
Copy link
Contributor

Thanks for raising this.

From a quick check I believe this is fixed by #617 but that fix introduces a related bug that dragging a recipe into a folder does not show up the change in the UI until you restart the program (almost certainly a signals issue that will be a small fix).

Once #617 is merged, I will have a look at fixing the drag-and-drop.

@matty0ung
Copy link
Contributor

Making progress. Fixed the drag-and-drop, but also confirmed that deleting a non-empty folder does still cause a crash even with the new database layer (#617). At first glance, I think it may be related to the fact that we delete the folder before we delete its contents (in BtTreeModel::deleteSelected). But my initial thoughts on such things often turn out to be wrong, so now investigating more deeply.

@mikfire
Copy link
Contributor

mikfire commented Mar 5, 2022

There isn't really a folder to delete and nothing is in the folder. The folder is a display layer trick, driven by an attribute on each object. If no object refers to a folder, that folder ceases to exist. It gets tricky with sub-folders.

If I were to make a random guess, check the signals being emitted and make sure all of the deletes are done before we finish the remove row operation.

@matty0ung
Copy link
Contributor

Thanks for the suggestion, will take a look.

matty0ung pushed a commit to matty0ung/brewken that referenced this issue Mar 6, 2022
matty0ung added a commit to Brewken/brewken that referenced this issue Mar 6, 2022
Stronger typing for BtTreeItem::Type, and fix for Brewtarget/brewtarget#622
matty0ung pushed a commit to matty0ung/brewtarget that referenced this issue Mar 6, 2022
@matty0ung
Copy link
Contributor

I believe this is fixed by the commits above. Please re-open if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants