Sign in to follow this  
Ezbez

How to safely let Python scripts interact with C++ objects that may be deleted

Recommended Posts

In my C++ game, I use boost.python for scripting purposes. I have an std::map<int, Entity*> in my C++ program. Each frame, I iterate through these Entities and run their OnFrame script. I also run scripts for collisions and keypresses. Sometimes, one Entity interacts with another, such as in collisions. In those cases, the Entity whose script is currently running will get an EntityHandle for another Entity. This EntityHandle can retrieve the Entity* from the map based on its index. The scripts can then modify the Entities from the EntityHandle. Entities can also delete themselves. Once this happens, they are removed from the std::map and deleted after all OnFrame scripts are run for this frame. My problem is fairly straight forward. Sometimes the game crashes. It's an unhandled exception: access violation reading memory at [somehex]. [sad] I know that it crashes during a Python script, when an Entity is accessed. It also only happens when something is deleted. Other than that, I haven't been able to find anything else out about the crash. This leads me to believe that it is caused by deleting an Entity, then trying to access it or something similar. Because of this, I would like suggestions as to how I can make the Python scripting fools-proof. Ideally, I'd like my program to never be caused to crash from a Python script. So how can I make sure that only entities that exist are being accessed? EntityHandles already return NULL if their index no longer exists in the std::map. Other than that, and checking for None in the Python code, I can't figure out how to tell if the entity has been deleted. Since a reference to an Entity in Python may be deleted without that script knowing, what can I do?

Share this post


Link to post
Share on other sites
Any threading going on?

Ie, is the Python script being run in a seperate thread than your C++ code that could delete Entities?

Your state of "about to be deleted" seems strange and troublesome, if it exists. When an Entity decides it should go away, is it immediately removed from the std::map, but the deletion is delayed until the end of the frame -- or is both the removal and deletion delayed?

How do you know it crashes in a Python script -- I assume via a stack trace?

How often are your std::map ints reused?

Can a script be talking to a perfectly fine Eintity in such a way that it makes it crash -- ie, are there Enitity types that don't like to be talked to in certain ways?

Are Python scripts allowed to store references to C++ Entities, or is all communication via a EntityHandle?

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Ezbez
This leads me to believe that it is caused by deleting an Entity, then trying to access it or something similar.
Don't delete the entities. If the bug disappears, then you'll know.
Quote:
Because of this, I would like suggestions as to how I can make the Python scripting fools-proof.
Use reference counting. Then if some place still refers to the object, say in Python script, it won't be deleted until that reference vanishes. Python uses reference counting for all its "own" objects. Then you wouldn't need the cumbersome handles either but you could simly use direct references to the reference counted objects. Also check out the weakref module if you're getting reference cycles.

Share this post


Link to post
Share on other sites
If a script is trying to manipulate an object that doesn't exist then it is a logical error in the program, similiar to trying to call a method that doesn't exist or getting the value of a variable that doesn't exist. To avoid the memory violation, the python wrapper could automatically check if the handle is still valid or not, and if it isn't it can throw a python exception instead of attempting to dereference an invalid pointer. The script could either check the validity of the handle before using it (look before you leap) or it could catch the python exception (better to beg forgiveness than ask permission). You could also design a system to register to receive an event when an object is deleted.

Share this post


Link to post
Share on other sites
>>Any threading going on?

Nope, none at all.

>>Your state of "about to be deleted" seems strange and troublesome, if it
>>exists. When an Entity decides it should go away, is it immediately removed
>>from the std::map, but the deletion is delayed until the end of the frame --
>>or is both the removal and deletion delayed?

It is both deleted and removed from the map simultaneously at the end of every frame.

>>How do you know it crashes in a Python script -- I assume via a stack trace?

Exactly, a stack trace.

>>How often are your std::map ints reused?

Indexes aren't ever reused. Unless you get so many Entity's that it wraps around.

>>Can a script be talking to a perfectly fine Entity in such a way that it makes
>>it crash -- ie, are there Entity types that don't like to be talked to in
>>certain ways?

Not sure what you mean by this, but I don't have different types of Entities. All differences are purely handled by the scripts.

>>Are Python scripts allowed to store references to C++ Entities, or is
>>all communication via a EntityHandle?

I don't have any ways to stop storing of a reference currently, but I don't and the problem still occurs. EntityHandles obtain a reference to the actual Entity and pass it to the script, so it could be stored. However, I only have used them for the duration of one script, so the referenced Entity shouldn't be deleted during a script.

>>Don't delete the entities. If the bug disappears, then you'll know.

It does disappear.

>>Use reference counting.

If I do this, how do I allow things to be deleted for other reasons? For example, I have bullets (yes, bullets get their own Entities). So that the program doesn't get too slow, I delete the bullets after a certain time period. Clearly reference counting wouldn't let me delete said bullet, since another script might not know when to discard its reference.

>>The script could either check the validity of the handle before using it (look before you leap)

I do check for the handle to be None (EntityHandle returns NULL if the index doesn't exist anymore), but the crash still happens.

Thanks for the replies.

Share this post


Link to post
Share on other sites
It sounds like you're doing the design fairly well, so it sounds like it might be a bug in how you retrieve an EntityHandle.

You shouldn't get the problems you are doing if an Entity can't be deleted while the script is running, and the EntityHandle correctly doesn't return a pointer to a deleted object.

Can you stop (or provide some form of debug output from) the program so you can see if the deleted entity is still in the std::map, or if the EntityHandle is somehow returning its address even though it isn't? That seems like the two points where the problem is likely to be, to me.

Share this post


Link to post
Share on other sites
What is the wrapper toolkit you are using to wrap your C++ objects?

Does any Python destructor call code into your C++ object?

I fear that the Python garbage collector might have a persisting reference to your C++ object. Using reference counting and having the python script attempt to drop all references to the object between script runs would solve the problem if this is the case.

You could also try to solve this by defensive programming techniques -- don't expose actual entities to python, instead expose a class that calls the methods for you. This guard class would observe the destruction of the object it is guarding, and refuse to forward messages afterwards. (and, most importantly, assert like crazy).

Share this post


Link to post
Share on other sites
>>It sounds like you're doing the design fairly well, so it sounds like it might
>>be a bug in how you retrieve an EntityHandle.

Possibly. Here's the code, it's pretty simple:

Entity* EntityHandle::Entity()
{
return level->GetEntity(index);
}

Entity* Level::GetEntity(int index)
{
std::map<int,Entity*>::iterator itr = entities.find(index);
if( itr == entities.end() )
return NULL;
else
return (*itr).second;
}


>>Can you stop (or provide some form of debug output from) the program so you
>>can see if the deleted entity is still in the std::map, or if the EntityHandle
>>is somehow returning its address even though it isn't? That seems like the two
>>points where the problem is likely to be, to me.

I've tried to do this, and haven't been particularly successful. However, I have put breakpoints on EntityHandle::Entity(), and can then check the index of all Entities requested. I was unable to find any ones that were problematic, but since I'd get them even when the problem wasn't happening, I'm not sure if I just didn't overlook it.

>>What is the wrapper toolkit you are using to wrap your C++ objects?

Boost.Python. If it helps, I use the return policy reference_existing_object for EntityHandle.Entity().

>>Does any Python destructor call code into your C++ object?

AFAIK, I have no 'Python destructors'. Could you clarify what you mean by that? The only things that I let Python store are EntityHandles, everything else is C++ side.

>>don't expose actual entities to python, instead expose a class that calls the
>>methods for you.

I like this idea. I will try it if no other options come up.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Ezbez
If I do this, how do I allow things to be deleted for other reasons? For example, I have bullets (yes, bullets get their own Entities). So that the program doesn't get too slow, I delete the bullets after a certain time period. Clearly reference counting wouldn't let me delete said bullet, since another script might not know when to discard its reference.
Look up the weakref module. If you use weakrefs, you clearly can delete the bullet by deleting its only real reference. You can think of weakref as a standard solution that replaces your custom EntityHandle. You might also change some flag in the object when it's supposed to be erased and let the scripts deal with deleting references to such objects; They probably need to handle the externally erased objects somehow anyway, e.g. detecting that a weakref returns null or in your old solution detecting that the handle is invalid. That's a good place to actually remove the reference. Set the variable holding the object to None or remove the object from a container.

Share this post


Link to post
Share on other sites
Does python ever have direct access to a pointer of type Entity (or a python wrapper around it), or does all access to an Entity fall through methods in EntityHolder and/or wrapper functions?

I'm guessing yes.

Does python ever have direct (or wrapped using python-wrappers) access to any data that is deleted when an Entity is destroyed?

To quote from Boost.Python documentation:
Quote:
A new Python object is created which contains a pointer to the referent, and no attempt is made to ensure that the lifetime of the referent is at least as long as that of the corresponding Python object. Thus, it can be highly dangerous to use reference_existing_object without additional lifetime management from such models of CallPolicies as with_custodian_and_ward. This class is used in the implementation of return_internal_reference.


Python objects are garbage collected. This means that their lifetime can persist for nearly any period of time beyond when the last reference goes away. I don't know what happens when the python wrapping class finally is destroyed, but I'd fear that it somehow pokes the wrapped C++ object, causing your memory error.

http://www.boost.org/libs/python/doc/v2/reference_existing_object.html

Share this post


Link to post
Share on other sites
Yes, Python does get access to a pointer to an Entity sometimes. I understand that this is dangerous, but I do not see how, in my particular case, it can cause an issue. I never store the pointer, only the EntityHandle, which appropriately checks to see if the Entity still exists. Thus, pointers are only used for the script during which the EntityHandle returned its (safe) result. Since no Entities are deleted during a frame, only afterwards, there is no possibility for the Entity to become invalid during a script.

Of course, my system does let the script writer hold onto the handle, so it is possible for that situation to arise, but it shouldn't currently, which makes me believe that there is another hidden problem here.

Share this post


Link to post
Share on other sites
Quote:
Original post by Ezbez
Yes, Python does get access to a pointer to an Entity sometimes. I understand that this is dangerous, but I do not see how, in my particular case, it can cause an issue. I never store the pointer, only the EntityHandle, which appropriately checks to see if the Entity still exists. Thus, pointers are only used for the script during which the EntityHandle returned its (safe) result. Since no Entities are deleted during a frame, only afterwards, there is no possibility for the Entity to become invalid during a script.

Of course, my system does let the script writer hold onto the handle, so it is possible for that situation to arise, but it shouldn't currently, which makes me believe that there is another hidden problem here.


Very well, let's repeat my theory:
Quote:
Originally posted by Me
Python objects are garbage collected. This means that their lifetime can persist for nearly any period of time beyond when the last reference goes away. I don't know what happens when the python wrapping class finally is destroyed, but I'd fear that it somehow pokes the wrapped C++ object, causing your memory error.

http://www.boost.org/libs/python/doc/v2/reference_existing_object.html


And again:
Garbage collection. Garbage collection means that the lifetime of a variable is not deterministic.

When a variable goes out of scope in Python, the Python garbage collector still knows about the variable. I don't know if this is your problem, I simply suspect it is your problem. But, in theory, when the Python garbage collector is deciding to dispose of the object, it might tell it that the Python reference is going away. This communication to the object could result in a crash in your application.

And, to repeat the link:
http://www.boost.org/libs/python/doc/v2/reference_existing_object.html
where it warns you that lifetime management has to be handled. You are not handling lifetime management, thus this could be why you are crashing.

I strongly suspect to get rid of this problem, using your existing pattern, you have to be using reference-based lifetime management of some kind, to allow the Python garbage collector the chance to be the last owner of your object.

I can't tell you how to fix this, because while I know some of how Python manages memory, I don't know Boost.Python to that depth.

I can, however, read the Boost.Python documentation, and it suggests a solution to your problem.

So, might I suggest a simpler implementation?

Create an object to pass to Python that is a wrapper around your objects. The Python script owns this object (return_value_policy<manage_new_object>) and has complete control over it's lifetime.

That wrapper's methods are "safe", in that they have indexes into the array and check on all calls that the array is still valid. The wrapper does not reference the array during destruction, nor does it communicate to anyone. if the wrapper's methods fail to find the indexed object, it asserts loudly.

The wrapper never ever exposes the wrapped class. Instead, it has an internal pointer that it forwards method calls through.

Now your PythonWrapper of your object has a lifetime determined by your Python scripting code. It also is designed to resist memory corruption: even if the Python script kept the object around, it would notice that it's index isn't valid, and assert rather than crash.

Your internal C++ objects, whose lifetime you want to explicitly control, are not exposed directly to Python. This gets rid of the python garbage collection system problems I think you are having.

That work?

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this