Outliner: Delete hierarchy crash #62098

Closed
opened 2019-03-01 20:41:39 +01:00 by Dalai Felinto · 10 comments

Blender Version
Broken: 491a98ca44
Worked: 491a98ca44~

Short description of error
Crash when deleting hierachy in the outliner

Exact steps for others to reproduce the error

  • Parent cube to camera.
  • In outliner, right-mouse-button the camera object > Delete Hierarchy (you need to change filter to filter out collections, in order to see the Delete Hierarchy option)

Or open the following file and RMB > Delete Hierarchy:
delete-hierarchy.blend

SUMMARY: AddressSanitizer: heap-use-after-free /home/dfelinto/src/blender/blender/source/blender/depsgraph/intern/depsgraph.cc:124 in DEG::Depsgraph::add_id_node(ID*, ID*)
Full backtrace: P920

**Blender Version** Broken: 491a98ca44 Worked: 491a98ca44~ **Short description of error** Crash when deleting hierachy in the outliner **Exact steps for others to reproduce the error** * Parent cube to camera. * In outliner, right-mouse-button the camera object > Delete Hierarchy (you need to change filter to filter out collections, in order to see the Delete Hierarchy option) Or open the following file and *RMB > Delete Hierarchy*: [delete-hierarchy.blend](https://archive.blender.org/developer/F6737582/delete-hierarchy.blend) `SUMMARY: AddressSanitizer: heap-use-after-free /home/dfelinto/src/blender/blender/source/blender/depsgraph/intern/depsgraph.cc:124 in DEG::Depsgraph::add_id_node(ID*, ID*)` Full backtrace: [P920](https://archive.blender.org/developer/P920.txt)
Author
Owner

Added subscriber: @dfelinto

Added subscriber: @dfelinto
Author
Owner

Added subscriber: @mont29

Added subscriber: @mont29

This issue was referenced by d1baed5e3d

This issue was referenced by d1baed5e3d03864b7968867fb93f9daa3d70428f
Author
Owner

Changed status from 'Open' to: 'Resolved'

Changed status from 'Open' to: 'Resolved'
Bastien Montagne was assigned by Dalai Felinto 2019-03-01 21:32:51 +01:00
Author
Owner

Bastien, you may want to look at this later. I just reverted the "fast delete multiple" method, since it was failing in the most basic of the cases. Better we fix it before make it the new default method.

Bastien, you may want to look at this later. I just reverted the "fast delete multiple" method, since it was failing in the most basic of the cases. Better we fix it before make it the new default method.

@dfelinto Can you please avoid reverting other people commits? Especially without any word of warning or anything? If we start reverting all commits generating crashes, we'll have to revert a lot of them… We try to FIX bugs, not just revert to blender 2.49 state! That kind of behavior should be strictly kept to critical issues (that one is not such one, unless we'd have been in release mode).

So next time, please just make the report, and let me fix it properly instead (unless you do fix it yourself). That’s the second time some non-critical issue is handled in a weird way in the tracker this week for me, am started to get a bit annoyed… to say the least…

For your info, that change was tested by me and user which initiated it, so it did pass some basic testing.

@dfelinto Can you please avoid reverting other people commits? Especially without any word of warning or anything? If we start reverting all commits generating crashes, we'll have to revert a lot of them… We try to **FIX** bugs, not just revert to blender 2.49 state! That kind of behavior should be strictly kept to critical issues (that one is not such one, unless we'd have been in release mode). So next time, please just make the report, and let me fix it properly instead (unless you do fix it yourself). That’s the second time some non-critical issue is handled in a weird way in the tracker this week for me, am started to get a bit annoyed… to say the least… For your info, that change was tested by me *and* user which initiated it, so it did pass some basic testing.

This issue was referenced by ccecc409e4

This issue was referenced by ccecc409e4eae160609bfd7e6803cda2afd0c0eb

This issue was referenced by 84820e7f58

This issue was referenced by 84820e7f5806a0dbdf609e9dbc1c2d33030f4dbb
Author
Owner

That kind of behavior should be strictly kept to critical issues (that one is not such one, unless we'd have been in release mode).

Hey Bastien, maybe we just agree to disagree here, because I see it quite differently. If a commit introduced a crash and can be reverted without breaking anyone's pipeline, I think it should reverted unceremoniously. Besides I do consider any crash a critical issue.

Either way, I would love to have a well established set of rules on how to behave in those cases to avoid running into unnecessary friction in the future. In fact I remember more than once we (as in Blender developers) talking about how we should not be shy on reverting broken commits. For real. But maybe it is my memory been biased.

The way I see this was really a no-brainer. On one hand I see the point of only reverting commits that introduced critical issues. On the other hand, I see no point of having a commit non-reverted if it brings no benefit and it introduces crashes. People are using 2.80 in production already, we all agreed on keeping it stable as much as possible. That said, sorry for bringing that annoyance to you, definitely not the intention here.

> That kind of behavior should be strictly kept to critical issues (that one is not such one, unless we'd have been in release mode). Hey Bastien, maybe we just agree to disagree here, because I see it quite differently. If a commit introduced a crash and can be reverted without breaking anyone's pipeline, I think it should reverted unceremoniously. Besides I do consider any crash a critical issue. Either way, I would love to have a well established set of rules on how to behave in those cases to avoid running into unnecessary friction in the future. In fact I remember more than once we (as in Blender developers) talking about how we should not be shy on reverting broken commits. For real. But maybe it is my memory been biased. The way I see this was really a no-brainer. On one hand I see the point of only reverting commits that introduced critical issues. On the other hand, I see no point of having a commit non-reverted if it brings no benefit and it introduces crashes. People are using 2.80 in production already, we all agreed on keeping it stable as much as possible. That said, sorry for bringing that annoyance to you, definitely not the intention here.

Crashes are serious issues, sure, and need to be handled swiftly. But:

  • Reverting commit that gets fixed and re-reverted few hours (or even days) later is only adding noise, nothing else.
  • If all crashes are critical, then we have tens of other critical issues in tracker, and again, we should revert to blender2.7x (or even 2.49) at once.
  • As a side note, 2.80 is not production ready, not released, and people are using it at their own risk. That’s the whole point of having releases. Despite the fact that we obviously always try to keep master as stable as possible.
  • Reverting someone else’s work without any notice nor any delay to let him/her fix the problem is rude. Even more so when said developer is usually highly active and reactive. Unless again it’s a critical situation (e.g. file corruption, about to release, etc.).
Crashes are serious issues, sure, and need to be handled swiftly. But: - Reverting commit that gets fixed and re-reverted few hours (or even days) later is only adding noise, nothing else. - If all crashes are critical, then we have tens of other critical issues in tracker, and again, we should revert to blender2.7x (or even 2.49) at once. - As a side note, 2.80 is not production ready, not released, and people are using it at their own risk. That’s the whole point of having releases. Despite the fact that we obviously always try to keep master as stable as possible. - Reverting someone else’s work without any notice nor any delay to let him/her fix the problem is rude. Even more so when said developer is usually highly active and reactive. Unless again it’s a critical situation (e.g. file corruption, about to release, etc.).
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#62098
No description provided.