edgar-mtz-e / slimdx

Automatically exported from code.google.com/p/slimdx
0 stars 0 forks source link

IncludeShim does not handle nested includes correctly (possible memory leak) #308

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
InlineShim (both DX9 and DX10 versions) won't handle nested includes
correctly. 

effect.fx:
#include "first.fxh"

first.fxh:
#include "second.fxh"

second.fxh:
...

Order of calls:
IncludeShim::Open (for first.fxh)
IncludeShim::Open (for second.fxh)

IncludeShim::Close(for second.fxh)
IncludeShim::Close(for first.fxh)

Currently the second call to Open will overwrite m_stream and m_handle with
new values. This causes several issues:
- the first call to Inline::Close will have the stream of second.fxh as an
argument, the second call to Inline::Close has nullptr as argument (the
first call has set m_stream to nullptr), so the client won't see the first
stream again
- the GCHandle that is allocated for first.fxh is never freed, because the
second call to Open overwrites the handle. => memory leak

I attached a patch that allows IncludeShim to handle arbitrary nesting.
Instead of storing m_stream and m_handle as member variables in
IncludeShim, IncludeShim now contains a stack of InlineFrames, each storing
the stream and the handle. A call to IncludeShim::Open pushes a new frame,
each call to IncludeShim::Close pops a frame. Furthermore, the d'tor of
IncludeShim will check for any IncludeFrames left in the stack and close
the streams/handles.

I've tested the D3D9 version, the D3D10 version is untested, so please test
(it's of course exactly the same implementation). 

Original issue reported on code.google.com by m...@andreloker.de on 25 Jul 2008 at 8:30

Attachments:

GoogleCodeExporter commented 9 years ago
Will fix.

Original comment by Mike.Popoloski on 26 Jul 2008 at 12:48

GoogleCodeExporter commented 9 years ago

Original comment by Mike.Popoloski on 26 Jul 2008 at 12:48

GoogleCodeExporter commented 9 years ago
Fixed.

Original comment by Mike.Popoloski on 26 Jul 2008 at 3:03