Ancillary objects not tracked in ObjectTable properly

[GoogleCodeExporter]

Ancillary objects, for example, a Surface created from a GetRenderTarget 
call, may not be tracked in the object table correctly.

If you have a Device (and no other SlimDX objects in the table), and you 
call GetRenderTarget(), Device will call Surface::FromPointer, which will 
call ObjectTable::Find, which will fail to find the object in the table. 
We will then gcnew a Surface, which will add us to the table.

However, if the underlying native call increments the COM reference count, 
we will be unable to release that reference count because the user doesn't 
know that the object should be disposed of -- they did not explicitly 
create it. 

I'm wary of returning to the old "dispose everything, everywhere" 
paradigm. Without looking too far into it, we may be able to go back there 
using the object table (we abandon our attempt to maintain only a single 
COM reference), but the implications are far reaching. 

However, I'm also not seeing any other obvious solutions. We could 
manually Release() immediately, relying on the fact that the underlying 
device will keep the interface around for us, but this is extremely 
brittle and may not work in a number of cases.

Original issue reported on code.google.com by josh.petrie on 26 Oct 2008 at 5:22

[GoogleCodeExporter]

OK, here's my idea for this. The problem, by itself, isn't a huge deal, because 
the
object only "leaks" once. After the first call to GetBackBuffer, the object 
will get
cached in our object table, and subsequent calls will just return that without 
upping
the reference count. The problem only occurs once the user needs to Reset or 
destroy
the device, in which case the object will still be lurking in the table.

Therefore, I propose that we mark said internal objects as needing to be 
cleaned up
by SlimDX, and call a Dispose internally on them whenever it is necessary 
(device
reset or what have you). Josh seems to like to call this "poor man's 
finalization".

Original comment by Mike.Popoloski on 26 Oct 2008 at 5:37

[GoogleCodeExporter]

A few more things. First, if we make this tracking option public, people could 
enable
it for any object they want and have it tracked down and released for them 
whenever
it's necessary. 

Josh suggested associating objects with other objects, so when one gets 
Disposed, any
child objects also get Disposed. Hell, we could even crawl the table and look 
for
objects that implement IResettable and deal with them during a Reset. Just 
things to
think about.

Finally, I attached our IRC conversation so we don't have to go begging MM 
later.

Original comment by Mike.Popoloski on 26 Oct 2008 at 5:52

Attachments:

• conversation.txt

[GoogleCodeExporter]

Hi, me again :)

After reading this I think I should give you some more information about what 
I'm
doing. It may be important to consider for your "poor mans finalization".

Some background:
Actually I'm building a prototype for an game engine that might be used for
commercial games (I'm CTO and programming games for the past 15+ years).

As we want to take the next step from C++ to C# for faster and less painful
development, I'm building a game engine with MOgre, SlimDX and some other useful
libraries.

Issue:
The DirectX device is initialized and managed by Ogre. Within the rendering 
loop I'm
obtaining the D3D device from Ogre as "IntPtr _device" and use it to initialize
SlimDX with "Device.FromPointer(_device)".

This is working extremely well, I can just save the state with 
CreateStateBlock, draw
some stuff (2D) myself and reset the device state from the saved state block 
later
before giving back control to the Ogre renderloop. But I need to restore 
anything on
the device to the same state before giving control back to Ogre. That's why I 
save
the rendertarget, my 2D library uses RTT a lot for sprites and the GUI.

In this engine Ogre is responsible for calling Reset() on the device whenever 
needed,
so I'm not calling the SlimDX implementation of Reset() after the device is 
lost.
Whenever Reset() is called I need to have all unmanaged DX resources released or
resetting the device will fail.

There should be a solution that does not depend on calling the SlimDX-Reset()
function for lost devices. I can live with a solution where I can notify SlimDX 
about
a lost device to release all unmanaged resources that may still be allocated.

One idea I had to solve the issue is an internal refcounter in SlimDX-ComObject 
that
is counting the refs that are not directly released. But after reading the blog 
I
realized that SlimDX user should never need to call some kind of release 
function for
the resources they obtain from the wrapper.

I just double checked it, using Dispose on the rendertarget object solves the 
issue
temporarily, not doing so results in an "unresettable" device. For the moment I 
can
live with that workaround, but for our final product I'd like to solve this 
issue
properly.

If I get any useful idea how to solve this, I'll post it here for you to 
consider.

Original comment by michael.stoyke@gmx.de on 26 Oct 2008 at 6:54

[GoogleCodeExporter]

You're stepping firmly into the "pathological use case" I mentioned concerning 
interop with other APIs using 
the same native subsystem. SlimDX itself can't know when some other API 
manipulates any of the underlying 
native state (for example, when Ogre resets the device) -- D3D itself can't 
really tell you this most of the 
time, and the times it can involve you polling for it.

However, if you know when the device needs to be reset, you can have the SlimDX 
ObjectTable tell you about 
all resources that are outstanding, and reset/dispose of those that need it. 
That is, I think, the best you can 
do here, if I understand you correctly.

As for this specific issue, I would prefer to avoid any solution that would 
involve us incrementing the COM 
reference count more correctly. Making the COM interactions cleaner makes the 
C# interactions more 
unpleasant: we'd have to have the client call some method similar to Dispose() 
(it wouldn't actually be 
Dispose() unless you explicitly new'd the reference) to decrement the reference 
count. We don't want to return 
to an API that violates the IDisposable guidelines, so we'd probably end up 
with a Release() method you'd 
need to call. This becomes really cumbersome for the clients because of the 
lack of scoped destruction (and 
thus, controllable RAII) in C# and the fact that we cannot leverage actual 
finalizers due to the threading 
concerns.

The category of objects affected by this bug are those that are internally 
created by a native interface, but 
exposed by that interface -- the back buffer, the implicit swap chain in D3D9, 
et cetera. That category is 
generally clearly called out in the documentation, so I am going to try adding 
an understanding of these 
special types of objects to the ObjectTable and see what comes up.

Original comment by josh.petrie on 26 Oct 2008 at 8:15

[GoogleCodeExporter]

Note that, if you really REALLY mean it, you can use Marshal.AddRef and 
Marshal.Release on our exposed COM pointer to alter the reference count 
manually. 
It's a rather risky thing to do, and best kept just for interop with something 
else 
that isn't handling ref counts correctly, but it is a valid option.

Remember, all of the other options for trying to track things that aren't 
explicitly 
created through SlimDX are attempting to match up with potentially rather 
complex 
and unpredictable situations. I don't think any of the attempts to track these 
internally created objects and automatically Dispose them will WORK in the 
general 
case. You might manage to fix some specific edge cases, but every time you do 
the 
rules for how to interact with SlimDX become more complex and harder to follow.

The documentation I'm working on discusses the pathological cases in more 
detail, so 
hopefully once that's finished and published, how to handle this will be a lot 
clearer. My recommendation is that any ancillary D3D object that is of interest 
to 
you should be created early in the initialization process as a SlimDX object 
and 
kept track of. So if you're using GetBackBuffer in your code, it should be 
called 
and stored internally as part of your initialization. This is by far the safest 
and 
easiest way to handle it, since now the Dispose points become stable and 
predictable.

And to answer your question Mike, you should not need to release the backbuffer 
to 
reset the device. 

Original comment by promit....@gmail.com on 26 Oct 2008 at 2:04

• Added labels: Type-Other, Usability
• Removed labels: Type-Defect

[GoogleCodeExporter]

I understand that my use of SlimDX as a wrapper for a D3D device that is managed
mostly by an external 3D engine is not the usual thing to do for most user.

If there is any way for me to query a list of unmanaged resources (like the
backbuffer reference in my case) that are created from SlimDX and not yet 
released
that would be perfectly ok for me.

As I can hook an event handler in the reset-device function of ogre, I can 
easily
query this list and release the resources myself. Or even better I could call 
some
kind of SlimDX.FreeUnmanagedResources function that could also be used by the 
SlimDX
Reset() function internally.

I only need to take care of resources that I created with the SlimDX interface, 
I'm
not trying to do something as complex as getting a resource from ogre and 
releasing
it through SlimDX. That would be a stupid thing to try :)

As far as I understand the ObjectTable in SlimDX these unmanged resources are 
also an
issue for user that are implementing a "pure" SlimDX application. I'm a little 
packed
with work at the moment but I want to write a small unit test class for this 
issue,
using only SlimDX, soon.

I can send you the test code when it's ready to use, I've noticed that you've 
already
started writing tests for SlimDX and I always like to contribute to opensource
projects that I'm using for my projects :)

Original comment by michael.stoyke@gmx.de on 26 Oct 2008 at 2:40

[GoogleCodeExporter]

You're right, it isn't just an multi library interop issue. It's a problem 
whenever 
you begin using objects that D3D creates internally, and unfortunately I don't 
think 
there's much that can be done about it. However, all of these objects ARE 
registered 
in the ObjectTable and can be iterated over, and you can get full callstacks 
for 
where they originate from as well. You can use that to debug and figure out 
exactly 
what objects are being stealth created, and then force creation at a specific 
time 
instead.

Hopefully that's an acceptable way to handle things?

Original comment by promit....@gmail.com on 26 Oct 2008 at 2:57

[GoogleCodeExporter]

Oh, and although your mixing of SlimDX and an external 3D engine is not a USUAL 
thing to do, it is a completely expected usage of SlimDX, and I've put 
significant 
effort (and will continue to do so) into making sure it works smoothly and 
correctly.

Original comment by promit....@gmail.com on 26 Oct 2008 at 3:00

[GoogleCodeExporter]

Thanks, I'll use the "iterate ObjectTable" approach.

Original comment by michael.stoyke@gmx.de on 26 Oct 2008 at 3:36

[GoogleCodeExporter]

I took a stab at the ancillary object tracking solution. The patch file is 
attached.

This is a quick hack to test that this can actually fix the problem (the Bug359 
unit 
test should pass). A more complete solution would plumb this notion of 
"ownership" 
between COM objects and their ancillary objects through the system more 
completly, 
replacing the exist Construct() method rather than overload ing. This hack only 
tests GetRenderTarget. 

I'm particularly interested in seeing the Bug359 fixture modified to include 
tests 
that simulate manipulating the reference count externally via the Marshal APIs, 
to 
see how we cope in those situations. I don't believe this existing 
implementation is 
a full solution, just a fix for the simple case where you are not interoping 
with 
other APIs.

Original comment by josh.petrie on 26 Oct 2008 at 6:01

Attachments:

• 359.patch

[GoogleCodeExporter]

Issue 420 has been merged into this issue.

Original comment by promit....@gmail.com on 29 Jan 2009 at 10:05

[GoogleCodeExporter]

Original comment by josh.petrie on 1 Feb 2009 at 8:54

• Changed state: Fixed