#1314 Lifecycle of pointers to 'concrete' sub-objects (C#-binding patch included)

open
nobody
None
5
2013-04-09
2013-04-09
Greg Knight
No

When retaining a reference to a 'concrete' object (as opposed to a pointer or reference) within another C++ class, the high-level wrapper of the container class may go out of scope while the high-level wrapper of the concrete remains within scope; the destruction of the high-level wrapper of the container class may destruct the low-level C++ object, causing subsequent accesses to the concrete wrapper to be corrupt. I originally observed this problem while iterating through a vector of rather more complex C++ objects in a C# foreach loop, while attempting to retain a small bit of data from one of them, leading to random crashes and corruption.

To illustrate, I've built an example:
- dog.h/dog.cxx: Groucho is a concrete inside of a Dog. Groucho knows the Answer.
- Program.cs: We instantiate a Dog so that we can ask Groucho for the Answer.
- dog.swig: Binds Groucho + Dog to C#

We query for the answer in two places: First, while we know the dog is whole and hale, and second, after we know the garbage collector has taken him. In the latter case, we reliably observe corruption of Groucho's answer. This corruption occurs because no valid reference to the original Dog remains in the CLR's heap, and the poor beast has been destructed.

I've attached a patch for the C# bindings (against swig-2.0.4, so needs refreshing) that make slightly different use of the HandleRef's Wrapper field, retaining the original Dog as Groucho's Wrapper in the Dog.Groucho property getter. With the attached patch, since C#-Groucho secretly retains a reference to the C#-Dog he's residing within, we don't observe the corruption/crash.

I've used the thus-patched binary extensively, but I'd like to know the SWIG team's perspective on this use case. The patch involves a memory-use risk, in that a given property getter causes the retrieved property to retain a hidden reference to the property owner, unless the typemap for the gotten property says otherwise. To illustrate this, a pointer to an unrelated dog, Lassie, is retained by the Dog class - observe the generated Lassie getter given the above patch.

It's sort of a question of whether to bloat or corrupt; however, ideally, I think we could detect whether we have a concrete (such as Groucho) and need to keep the parent object alive, or if we have a pointer/reference (such as Lassie) for whom Wrapper should just be 'this'. I was not immediately clear on how to achieve this based on cursory inspection of the SWIG code, but may have missed something.

Thoughts? Shall I modernize and test the patch for submission, or do you have recommendations as to how I could fix this in a way less likely to risk bloat in some use cases?

1 Attachments

Discussion


Log in to post a comment.