coin3d / pivy

python bindings to coin3d
ISC License
53 stars 37 forks source link

Recursion in SoBaseKit.__getattr__ #85

Open sebjf opened 2 years ago

sebjf commented 2 years ago

Hi,

I've been experimenting with the draggers in FreeCAD and encountered a (possibly) infinite recursion bug when using debugpy with pivy.

It seems to be that SWIG attempts to get the attribute 'this' from SoBaseKit, but the __getattr__ implementation unconditionally attempts to retrieve the catalog (SoBaseKit_getNodekitCatalog), which itself attempts to get this, and so on. The error seems to be silently handled inside Pivy but becomes a problem when using the debugger.

There's further analysis by the debugpy team.

Adding a case as for __setattr__ that returns the SoNode implementation resolves this in the tests I've done.

def __getattr__(self,name):
       if name == 'this':
          return SoNode.__gettattr__(self,name)
       c = _coin.SoBaseKit_getNodekitCatalog(self)
       if c.getPartNumber(name) >= 0:
          part = self.getPart(name,1)
          return part
       return SoNode.__getattr__(self, name)

Though I don't know enough about SWIG to be sure this is the correct approach.

looooo commented 2 years ago

Thanks for reporting! It's difficult for me to understand the issue. But I tested the solution and didn't find any issue, so from my point we can add the suggested solution. Maybe @tuhtah @wwmayer or @GerhardR have a look at this.

GerhardR commented 2 years ago

wow, what an honor to be called upon to look at Pivy code :)

from looking at the code and my memory of OpenInventor and Pivy, the modification in SoBaseKit.i is there to make the parts of a nodekit accessible in a Pythonic way (i.e.. kit.part etc).

so the proposed fix should be correct and probably the problem was found already before in setattr.

looooo commented 2 years ago

wow, what an honor to be called upon to look at Pivy code :)

What an honor you are actually responding to my request :) I didn't expect this. Thanks a lot.

@sebjf do you want to create a PR for the proposed solution?

sebjf commented 2 years ago

@sebjf do you want to create a PR for the proposed solution?

Done! Thanks for considering the issue so quickly! :)