Discussion:
[Opensg-users] [OpenSG 2.x] Bug fixes
Johannes Brunen
2015-02-06 13:15:01 UTC
Permalink
Hello Carsten and Gerrit,



back to OpenSG :-)



Attached you can find a patch file for some errors I have found. I have
created the patch file against the current master.



Could you please review my corrections and check them into the git
repository if they are fine.



I have found the following problems:

1. OSGShaderCache: Usage of incorrect end iterator types

2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.

3. OSGShaderCacheTree.inl: Minor issue with the dumpDot function.

4. OSGShaderProgamVariableChunk.cpp: Imho, the resolveLinks function
should clear the _mfDestroyedFunctors container (?).



Best,

Johannes
Carsten Neumann
2015-02-06 15:33:33 UTC
Permalink
Hello Johannes,
Post by Johannes Brunen
back to OpenSG :-)
welcome back :)
Post by Johannes Brunen
Attached you can find a patch file for some errors I have found. I have
created the patch file against the current master.
Thank you for the bug fixes!
Post by Johannes Brunen
1. OSGShaderCache: Usage of incorrect end iterator types
applied.
Post by Johannes Brunen
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
I'll hold off applying this one for a couple of days so that either
Gerrit has a chance to comment or I can read up on how the cache works.
From you description it sounds like you have found the source of some
mysterious (and hard to find) bugs that have come up over time. Thanks
for finding this one!
Post by Johannes Brunen
3. OSGShaderCacheTree.inl: Minor issue with the dumpDot function.
applied.
Post by Johannes Brunen
4. OSGShaderProgamVariableChunk.cpp: Imho, the resolveLinks function
should clear the _mfDestroyedFunctors container (?).
Agreed, it should be cleared. Applied.

Cheers,
Carsten
Gerrit Voß
2015-02-11 07:33:43 UTC
Permalink
Hi,
Post by Carsten Neumann
Hello Johannes,
Post by Johannes Brunen
back to OpenSG :-)
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
I'll hold off applying this one for a couple of days so that either
Gerrit has a chance to comment or I can read up on how the cache works.
From you description it sounds like you have found the source of some
mysterious (and hard to find) bugs that have come up over time. Thanks
for finding this one!
I'll try to find some time to go over this one, but it might be more
towards this weekend.

kind regards
gerrit
Johannes
2015-02-23 07:51:27 UTC
Permalink
Ping :-)
Post by Gerrit Voß
Hi,
Post by Carsten Neumann
Hello Johannes,
Post by Johannes Brunen
back to OpenSG :-)
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
I'll hold off applying this one for a couple of days so that either
Gerrit has a chance to comment or I can read up on how the cache works.
From you description it sounds like you have found the source of some
mysterious (and hard to find) bugs that have come up over time. Thanks
for finding this one!
I'll try to find some time to go over this one, but it might be more
towards this weekend.
kind regards
gerrit
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
Johannes
2015-03-03 09:34:50 UTC
Permalink
Hello Gerrit and Carsten,

I really would like to close this case. Did you have any time to look
into my patch? Hopefully, I'm not pestering to much.

Kind regards,
Johannes
Post by Gerrit Voß
Hi,
Post by Carsten Neumann
Hello Johannes,
Post by Johannes Brunen
back to OpenSG :-)
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
I'll hold off applying this one for a couple of days so that either
Gerrit has a chance to comment or I can read up on how the cache works.
From you description it sounds like you have found the source of some
mysterious (and hard to find) bugs that have come up over time. Thanks
for finding this one!
I'll try to find some time to go over this one, but it might be more
towards this weekend.
kind regards
gerrit
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
Gerrit Voß
2015-03-04 20:11:11 UTC
Permalink
Hi,
Post by Johannes
Hello Gerrit and Carsten,
I really would like to close this case. Did you have any time to look
into my patch? Hopefully, I'm not pestering to much.
not, I have to apologize, got stuck in some excel sheets and other fun
things ;).
Post by Johannes
Hi,
Post by Carsten Neumann
Hello Johannes,
Post by Johannes Brunen
back to OpenSG :-)
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
for my understanding, what happens is you sub some cache entries and
later on Cache::clear the system tries to delete them again. Your patch
looks ok, though I'm still not sure where the subtle difference is ;).

Is it easy to dump a dot file for this case.

kind regards
gerrit
Johannes
2015-03-05 09:47:36 UTC
Permalink
Hello Gerrit,

good to here from you :-)
Post by Gerrit Voß
Post by Johannes Brunen
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
for my understanding, what happens is you sub some cache entries and
later on Cache::clear the system tries to delete them again. Your patch
looks ok, though I'm still not sure where the subtle difference is ;).
Is it easy to dump a dot file for this case.
No it is not :-( The model with which I have worked on the issue was
rather big and I did have a complicated procedure list to reproduce the
error. I was able to find a way to get imho to the cause of the problem
in function 'void ShaderCacheTreeV3<ObjectT, LevelBits>::sub(UInt32
uiIdx)' but can't remember the steps which have taken me to this place
in detail. The problem is the missing '0x0000 != (i & uiCurrBits) &&'
part in the second 'if' statement. This leads to remaining entries in
the cache which do pop up later on.

I do have some dot files and logging of instrumented code of the
original case. But I'm not sure that this information is of any help for
you.

Attached you find what I do have. The session goes from program start to
crash. I did have the code somehow instrumented by adding member
'_oDbg_store' in the ShaderCache. I fill this set in the
'updateRemoveCallback()' function for each 'addDestroyedFunctor' call.
The crash happens since this set is not empty in the ShaderCache destructor:

ss << "ShaderCache::dtor = " << std::hex << this << " still in
_oDbg_store = " << *iter;

For this reason, the living objects carrying the callback
'ShaderCache::removeShaderVar' due to the 'addDestroyedFunctor' calls,
might execute the callback in the already destroyed ShaderCache object.
After inspection of the code I have found that the 'removeShaderVar'
function do not remove all entries in the '_oVarTree.sub(uiVarId);' call.

I hope I could give you enough information to tackle the issue.

Best,
Johannes
Gerrit Voß
2015-03-06 07:47:13 UTC
Permalink
Hi,
Post by Johannes
Hello Gerrit,
good to here from you :-)
Post by Gerrit Voß
Post by Johannes Brunen
2. OSGShaderCacheTree.inl: The ShaderCacheTreeV3::sub() method is imho
incorrect. The ShaderCache::clear() function currently does not work
properly. I was facing references to already destroyed ShaderCache
objects from ShaderProgramVariableChunks.
for my understanding, what happens is you sub some cache entries and
later on Cache::clear the system tries to delete them again. Your patch
looks ok, though I'm still not sure where the subtle difference is ;).
Is it easy to dump a dot file for this case.
No it is not :-( The model with which I have worked on the issue was
rather big and I did have a complicated procedure list to reproduce the
error. I was able to find a way to get imho to the cause of the problem
in function 'void ShaderCacheTreeV3<ObjectT, LevelBits>::sub(UInt32
uiIdx)' but can't remember the steps which have taken me to this place
in detail. The problem is the missing '0x0000 != (i & uiCurrBits) &&'
part in the second 'if' statement. This leads to remaining entries in
the cache which do pop up later on.
ok, I see, that was the bit I missed ;(.
Post by Johannes
I do have some dot files and logging of instrumented code of the
original case. But I'm not sure that this information is of any help for
you.
Attached you find what I do have. The session goes from program start to
crash. I did have the code somehow instrumented by adding member
'_oDbg_store' in the ShaderCache. I fill this set in the
'updateRemoveCallback()' function for each 'addDestroyedFunctor' call.
ss << "ShaderCache::dtor = " << std::hex << this << " still in
_oDbg_store = " << *iter;
For this reason, the living objects carrying the callback
'ShaderCache::removeShaderVar' due to the 'addDestroyedFunctor' calls,
might execute the callback in the already destroyed ShaderCache object.
After inspection of the code I have found that the 'removeShaderVar'
function do not remove all entries in the '_oVarTree.sub(uiVarId);' call.
I hope I could give you enough information to tackle the issue.
thanks, I committed the patch as it looks fine. I'll continue to look
at your test cases, just in case there is something else lurking
around ;).

kind regards
gerrit

Loading...