#619 Made TreeView stable

[realthunder]

I caught the crash in debugger.

Yep, stack frame 26, Tree.cpp:1006, the assertion failure. I've removed that assertion. Please sync and try again.

[DeepSOIC]

Yep, stack frame 26, Tree.cpp:1006, the assertion failure. I've removed that assertion. Please sync and try again.

Done. No crashes so far...

[DeepSOIC]

Good news. I lowered PoM's timer interval from 300 to 30 ms. This made it hit the assert every time. With your latest change, no asserts, and no crashing. Works fine!

[DeepSOIC]

This causes an endless loop:

Code: Select all

class proxy(object):
    def __init__(self, obj):
        obj.Proxy = self

class vpproxy(object):
    def __init__(self,vobj):
        vobj.Proxy = self

    def attach(self, vobj):
        self.ViewObject = vobj
        self.Object = vobj.Object

    def claimChildren(self):
        return [self.Object]

if not App.ActiveDocument:
    App.newDocument()

obj = App.ActiveDocument.addObject("App::FeaturePython", "chicken_or_egg")
proxy(obj)
vpproxy(obj.ViewObject)

I was quite surprised to find out it doesn't eat up more and more memory. Just hangs.

[DeepSOIC]

And another example with same result, that can actually happen for real.

Code: Select all

if not App.ActiveDocument:
    App.newDocument()

import Part
chicken = App.ActiveDocument.addObject("Part::Extrusion", "chicken")
egg = App.ActiveDocument.addObject("Part::Extrusion", "egg")
chicken.Base = egg
egg.Base = chicken

Hangs, too.

[realthunder]

And another example with same result, that can actually happen for real.

Hmm... It's easy for me to fix this up by extra checking, and just not create item for this kind of scenario. But it can't prevent cyclic dependency, and it is still there. There bound to be other problems, e.g. if DAGView is enabled, it doesn't freeze the GUI, but spits error message endlessly. To be fair, FC without my patch will lock up too.

It is actually a deeper problem than it appears. It is reasonable to demand that FC shall guarantee no cyclic dependency when creating models using GUI, but what about using python? None of the programming language that I know of prevent you from locking yourself up with infinite loop. Is it worth the trouble to detect such thing, and to what extent? Anyway, if checking is desirable, then it is better done in App namespace, before it reaches GUI, probably in link property assignment.

[DeepSOIC]

To be fair, FC without my patch will lock up too.

Indeed it does, I haven't checked this . But it shouldn't. I was using an artificial circular dependency in Lattice2 as a method to temporarily block recomputes...

People arrive to cyclic dependencies in projects quite often, and it should be possible to fix the loop. Also, the loop may just be a temporarily state when some operation is in progress, like OpenSCAD ReplaceObject.

I'll have a look at what's going on, because I now have a feeling it's not your code that hangs.

[realthunder]

I'll have a look at what's going on, because I now have a feeling it's not your code that hangs.

It is kinda of. It actually hangs inside Qt::TreeWidgetItem.addChild. They don't bother checking cyclic reference either. And for a good reason, as it may be a potentially expensive operation.

I added the check nevertheless. We'll see how it affects FC performance. Please sync the branch. As I mentioned, it may cause other problems. If you enable DAGView, you'll be spammed with endless error message.

[DeepSOIC]

I checked down to:
OS: Windows 8
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.16.6219 (Git)
Build type: Release
Branch: FreeCAD-DeepSOIC6
Hash: 1aff8d612e303501d401891e75aae8680b8c130f
Python version: 2.7.8
Qt version: 4.8.6
Coin version: 4.0.0a
OCC version: 6.7.1
And it hangs too. So we either were lucky and cyclic dependencies made so far never involved cyclic tree. Or they were, but people couldn't reproduce it, so didn't bother reporting.

If you enable DAGView, you'll be spammed with endless error message.

Well, at least it doesn't block one from fixing the loop

I added the check nevertheless. We'll see how it affects FC performance. Please sync the branch.

It may, as it looks like tree update is fired for every change to a property... So maybe just leave it alone for now?

[realthunder]

It may, as it looks like tree update is fired for every change to a property... So maybe just leave it alone for now?

The cyclic check only happens when a free item (i.e. an item at document root) is newly claimed, which already has children. The check needs to visit all its children. That's the only case requiring a check. We'd be creating new items (without children obviously) for all other cases. So, I think the performance impact should be limited.