← Back to team overview

kicad-developers team mailing list archive

Re: Deletion in plugins causing trouble

 

thank you all.

Yes, looking at other projects, blender in particular, would be wise. my
perception is they have a really nice scripting capability. You can do
anything there in scripting including commands that are indistinguishable
from the others.

I don't suppose anyone knows a blender contributor? (or some other open
project with strong scripting support).


Miles

personal reference: patch 6/7




On Thu, Mar 8, 2018 at 5:35 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> As a short term fix, I'll allow this patch to prevent the segfaults.
> However, I do not want to continue to allowing these types of ad hoc
> fixes.  We should take Dick's suggestion and look at what other projects
> are doing.  We cannot be the only project wrestling with this issue.
>
> On 3/8/2018 3:59 AM, Maciej Sumiński wrote:
> > In my opinion #3 is acceptable. It does not require any extra changes in
> > the existing scripts and it fixes the problem until we come up with a
> > better scripting interface. If nobody has objections, I am going to
> > commit the patch.
> >
> > Regards,
> > Orson
> >
> > On 03/06/2018 10:02 AM, miles mccoo wrote:
> >> Thanks all for your replies.
> >>
> >> I like the plugin mechanism. It does some nice things for python folks.
> >> Refresh, undo, garbage collection (I think). I want to see it succeed.
> >>
> >> From reading Orson's mail, I think I wasn't entirely clear, so let me
> try
> >> to fix that first.
> >>
> >>
> >> The plugin mechanism tries to track the changes of a python plugin. In
> the
> >> c++ world, developers are expected to inform BOARD_COMMIT that something
> >> has changed. The plugin infrastructure does this for you. One could
> compare
> >> it to garbage collection. My interpretation of how this is done is
> simple:
> >> before a plugin runs, make a list of all design objects. zones, tracks,
> >> modules, nets... after the plugin completes, do a "diff".
> >>
> >> This is mostly fine with the exception of removed items.
> >>
> >> The problem with removed items is how do you do a diff on a deleted,
> memory
> >> managed, cleaned up object? Perhaps that memory now contains a different
> >> kind of object. Perhaps it's been reallocated to a similar object as
> before.
> >>
> >>
> >>
> >> *as it stands now, any plugin that removes items from a board item
> >> container will likely fail*. It's not quite a crash, but it has a
> similar
> >> feel to the user.
> >>
> >>
> >>
> >> Solutions. I can think of four.
> >>
> >> solution 1. Why is the plugin mechanism in the business of tracking
> >> changes? Shouldn't BOARD_COMMIT just magically happen whenever a c++
> >> instance is modified. I brought this up in a previous thread[1] and
> various
> >> reasons were given why this isn't desirable.
> >>
> >> solution 2. Same as #1 except BC magic only happens on python API calls.
> >> The plugin mechanism would no longer do diffs. Just add BC checkpoints.
> >> This is probably a lot of work. I might be willing to do this work but
> it
> >> would take time. [2]
> >>
> >> solution 3. easy to implement. turn off deletion on calls to remove if
> >> currently running a plugin
> >>
> >>    - plugin gets a boolean: isPluginActive. set/unset around the call to
> >>    actual python run code.
> >>    - add pcbnew.isPluginActive() to python api. (I could imagine this
> could
> >>    have other uses)
> >>    - the remove code looks at isPluginActive to decide whether to set
> >>    isown. (that code is actually python, not swig)
> >>
> >> I have #3 implemented and my plugins no longer crash. *See attached
> patch
> >> for if folks agree it is an acceptable stopgap.*
> >>
> >> solution 4. probably not the direction I would go but a way to enable
> >> python memory management and do the plugin diff thing. Basically, give
> each
> >> object some sort of unique id. (could be useful for other things). In
> >> addition to holding a list of object pointers, plugin infrastructure
> would
> >> hold a list of object ids.
> >>
> >>
> >> *Given the ease with which a plugin can hit this issue and given how
> long
> >> it would take to get #2 right, I guess I'm recommending the changes of
> that
> >> attached patch for #3.*
> >>
> >> Miles
> >>
> >> [1] https://lists.launchpad.net/kicad-developers/msg32063.html
> >> [2] I think my personal opinion is that #1 is superior over #2. Python
> >> people shouldn't have to care but why is it that c++ should or want to?
> >> Yes, I read the arguments from the previous thread but I'm not
> convinced.
> >> I'm also just a kicad spectator, so there's a lot I don't know.
> >>
> >>
> >>
> >>
> >>
> >> On Fri, Mar 2, 2018 at 2:47 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
> >> wrote:
> >>
> >>> LOL, I just replied to Miles.  Thanks Orson for helping out!
> >>>
> >>> On 3/2/2018 8:36 AM, Maciej Sumiński wrote:
> >>>> Hi Miles,
> >>>>
> >>>> I suppose the silence in the thread indicates there are not many
> >>>> developers knowing the Python scripting interface inside out. Since
> you
> >>>> are both studying the scripting interface and developing own scripts,
> it
> >>>> is quite possible you are the most competent person to give us an
> advice
> >>>> on how to proceed. See some comments below, but I am neither a Python
> >>>> script developer nor a scripting interface maintainer, so I might be
> >>>> lacking some basic knowledge here.
> >>>>
> >>>> On 02/28/2018 05:12 PM, miles mccoo wrote:
> >>>>> So I'm plugin-ifying my python scripts (the mechanism is awesome).
> One
> >>> of
> >>>>> the plugins deletes some stuff and that is causing trouble.
> >>>>>
> >>>>>
> >>>>>
> >>>>> I'm not sure how to fix the root cause. Hence this mail.
> >>>>>
> >>>>>
> >>>>>
> >>>>> The plugin just deletes Edge.Cuts[1]:
> >>>>> for d in board.GetDrawings():
> >>>>>             if (d.GetLayerName() == 'Edge.Cuts'):
> >>>>>                 board.Remove(d)
> >>>>>
> >>>>>
> >>>>>
> >>>>> in board_item_container.i, I see this (with stuff deleted):
> >>>>> %rename(RemoveNative)    BOARD_ITEM_CONTAINER::Remove;
> >>>>>     def Remove(self,item):
> >>>>>         self.RemoveNative(item)
> >>>>>         item.thisown=1
> >>>>>
> >>>>>
> >>>>> Setting thisown tells, python "you own it". Delete it when you're
> done.
> >>>>> Which it does.
> >>>>>
> >>>>>
> >>>>> The problem this causes is that the plugin mechanism saves a list of
> all
> >>>>> objects before running the plugin and then checks if any of them has
> a
> >>> null
> >>>>> list after (ie is it still in the design).
> >>>>
> >>>> Is this mechanism implemented to handle memory leaks? If so, would not
> >>>> it be sufficient to stick to 'thisown' flag on Remove() calls or is
> >>>> there another way objects might be destroyed using Python scripts?
> >>>>
> >>>>> Since the object has been deleted by python, the plugin stuff gets
> >>> confused.
> >>>>>
> >>>>>
> >>>>> *So, the question is how to fix this?*
> >>>>>
> >>>>>
> >>>>> It appears that the plugin infrastructure will delete for you (that's
> >>> what
> >>>>> I'm guessing), so the thisown setting shouldn't be done.
> >>>>>
> >>>>>
> >>>>> On the other hand, if running code from within a standalone script
> (ie
> >>> from
> >>>>> regular python shell), now thisown'ing it would yield a memory leak.
> >>>>>
> >>>>>
> >>>>>
> >>>>> Perhaps the plugin stuff should have some sort of flag indicating
> >>> "you're
> >>>>> in a plugin". Then the thisown setting could be conditional.
> >>>>
> >>>> If the object listing mechanism is required for other reasons, then I
> >>>> suppose it is second best idea. Generally speaking, I would like to
> make
> >>>> the scripting interface convenient for the users, so they do not need
> to
> >>>> worry about whether their scripts are run standalone or as a plugin.
> >>>> Let's hide the dirty magic from them and make the coding process
> >>> enjoyable.
> >>>>
> >>>> Regards,
> >>>> Orson
> >>>>
> >>>>> But I'm just a spectator. *I'm happy to put in the time to fix this
> but
> >>>>> need guidance on what approach to take.*
> >>>>>
> >>>>>
> >>>>> Miles
> >>>>>
> >>>>>
> >>>>>
> >>>>> [1] full plugin text
> >>>>> import pcbnew
> >>>>>
> >>>>> class RemoveBoundaryPlugin(pcbnew.ActionPlugin):
> >>>>>     def defaults(self):
> >>>>>         self.name = "Remove boundary"
> >>>>>         self.category = "A descriptive category name"
> >>>>>         self.description = "This plugin reads a dxf file and converts
> >>> it to
> >>>>> a zone"
> >>>>>
> >>>>>     def Run(self):
> >>>>>         board = pcbnew.GetBoard()
> >>>>>
> >>>>>         for d in board.GetDrawings():
> >>>>>             print("{}".format(str(d)))
> >>>>>             #print("on layer {} {} {}".format(d.GetLayerName(),
> >>>>>             #                                 str(d.GetStart()),
> >>>>>             #                                 str(d.GetEnd())))
> >>>>>             if (d.GetLayerName() == 'Edge.Cuts'):
> >>>>>                 board.Remove(d)
> >>>>>
> >>>>> RemoveBoundaryPlugin().register()
> >>>>>
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> Mailing list: https://launchpad.net/~kicad-developers
> >>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>>> Unsubscribe : https://launchpad.net/~kicad-developers
> >>>>> More help   : https://help.launchpad.net/ListHelp
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> Mailing list: https://launchpad.net/~kicad-developers
> >>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>> Unsubscribe : https://launchpad.net/~kicad-developers
> >>>> More help   : https://help.launchpad.net/ListHelp
> >>>>
> >>>
> >>>
> >>> _______________________________________________
> >>> Mailing list: https://launchpad.net/~kicad-developers
> >>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>> Unsubscribe : https://launchpad.net/~kicad-developers
> >>> More help   : https://help.launchpad.net/ListHelp
> >>>
> >>
> >>
> >>
> >> _______________________________________________
> >> Mailing list: https://launchpad.net/~kicad-developers
> >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >> Unsubscribe : https://launchpad.net/~kicad-developers
> >> More help   : https://help.launchpad.net/ListHelp
> >>
> >
> >
> >
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help   : https://help.launchpad.net/ListHelp
> >
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>

Follow ups

References