Creating a dependency loop of containers crashes FreeCAD

[DeepSOIC]

Hi!
I found an interesting way to crash FreeCAD.
Run this:

Code: Select all

if App.ActiveDocument is None:
    App.newDocument()

part1 = App.ActiveDocument.addObject("App::Part", "Part1")
part1.Group = [part1]

This itself doesn't cause a crash, but adding another object after doing this does.

Doing a part1->part2->part1 dependency loop causes an out-of-memory exception, and then crashes FreeCAD too.

This problem situation happened when I was modeling with Part-o-magic enabled, and Part-o-magic made such a dependency loop. Of course it is Part-o-magic problem, but it is a FreeCAD problem too, I think.

I lost quite a bit of progress on my model, unfortunately.

OS: Windows 10
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.17.7668 (Git)
Build type: Release
Branch: BodyBase3
Hash: ef872d37a8a365aeb04567d80616b28a826ed41c
Python version: 2.7.8
Qt version: 4.8.7
Coin version: 4.0.0a
OCC version: 6.8.0.oce-0.17

[microelly2]

It makes sense to think about a FreeCAD-Database connection, where dependency structures can be stored.

For some reasons databases are faster and can bring more comfort than the FreeCAD DAG model.
I think using the FreeCAd webserver interface can be an opportunity.

I have played with networkx. Storing Data in FreeCAD and sending them with tasks to specialized servers is for me a good way.

But how to prevent users(programmers) to run into a stackoverflow?
Maybe there is a way to analyse scripts to detect recursive call sequences.
But I'm afraid this is not computable.

[ickby]

Doing a part1->part2->part1 dependency loop causes an out-of-memory exception, and then crashes FreeCAD too.

This problem situation happened when I was modeling with Part-o-magic enabled, and Part-o-magic made such a dependency loop. Of course it is Part-o-magic problem, but it is a FreeCAD problem too, I think.

I think the problem is not that it does not work, as I don't know of any way that it should work, but the crash instead of a warning. Am I getting you right here?

It makes sense to think about a FreeCAD-Database connection, where dependency structures can be stored.

How does that relate to the given problem?

[DeepSOIC]

I think the problem is not that it does not work, as I don't know of any way that it should work, but the crash instead of a warning. Am I getting you right here?

Yes, it should not crash, so that the situation can be fixed if it happens to arise because of whatever error in coding (think add-on workbenches and macros, especially).

[ickby]

yes definitely something to take care of. Could you add a ticket, so that it is not forgotten?

[DeepSOIC]

Could you add a ticket, so that it is not forgotten?

Done issue #2567

[wmayer]

Code: Select all

bool DocumentObjectGroup::hasObject(const DocumentObject* obj, bool recursive) const
{
    const std::vector<DocumentObject*>& grp = Group.getValues();
    for (std::vector<DocumentObject*>::const_iterator it = grp.begin(); it != grp.end(); ++it) {
        if (*it == obj) {
            return true;
        } else if ( recursive && (*it)->isDerivedFrom (App::DocumentObjectGroup::getTypeId()) ) {
            App::DocumentObjectGroup *subGroup = static_cast<App::DocumentObjectGroup *> (*it);
            if (subGroup->hasObject (obj, recursive)) {  // <<<<==============    This causes the overflow
                return true;
            }
        }
    }

    return false;
}

In this case 'SubGroup' is identical to 'this'. So, the problematic line must be changed to:

Code: Select all

if (subGroup != this && subGroup->hasObject (obj, recursive)) {

[DeepSOIC]

In this case 'SubGroup' is identical to 'this'. So, the problematic line must be changed to:

Code: Select all

if (subGroup != this && subGroup->hasObject (obj, recursive)) {

Feels like this won't stop the recursion in case of part1->part2->part1 loop.

[wmayer]

Then a private overloaded method

Code: Select all

bool DocumentObjectGroup::hasObject(const DocumentObject* obj, bool recursive, std::set<DocumentObject*>&) const

can be implemented. Internally it adds 'this' and for recursive calls it checks whether 'subGroup' is already. The public method hasObject has to use the private version.

[Kunda1]

Soft bump to check the status of issue #2567