Hi Gerrit,
Gerrit Voß wrote:
> On Sat, 2009-06-13 at 12:12 -0500, Carsten Neumann wrote:
>> hm, now I'm wondering if perhaps I was misunderstanding what
>> createLocal() does. Can you explain please? Specifically:
>> - if I createLocal an object in aspect N and sync to aspect M, will
>> there be an aspect copy of the object in M, i.e. can there be a
>> meaningful pointer to the object in aspect M ?
>
> not really unless you are willing to accept global objects which are
> the same for all aspects, require different access protection and
> take care that their pointers don't jump aspects.
ugh, does not sound pleasant to me... ;)
>> - what is the intended use, i.e. what things should be created with
>> createLocal() and what does that buy us?
>
> One major reason was to build application objects with OpenSG. So inside
> OpenSG itself they aren't of much use. E.g. their main occurrence is in
> ComplexSceneManager.
ok, so we probably need to get rid of a bunch of uses in the system, the
patch I sent with the last mail did that for the internal chunks of
Simple{,Textured}Material and the State object in PrimaryMaterial. I
remember a bunch of uses of createLocal() in the Stages, that I need to
think about whether they are safe.
>>> - image::change does not check if the parent ptr is NULL. This
>>> happens because the weak ptr returns NULL as the createThreads
>>> object goes out of scope.
>> I don't see why Image::changed has to check the parent pointer (they are
>> not weak pointers, they are uncounted, thus they can not suddenly become
>> NULL the way weak pointers can), because if the parent goes out of scope
>> it should remove itself from being a parent to the image. That does not
>> seem to happen, is that the real bug here?
>
> ah, ok we did not change this.
well, we did change it, but in the opposite direction, parent pointer
being weak was the initial implementation, we changed it to uncounted ;)
> So than the old thing happens that NULL is synced.
yes.
>>> - there is a chance for a race condition when image calls
>>> parent->changed and the createThread has the parent run resolveLinks.
>>> But in this case this should not be a problem as parent::changed and
>>> parent::resolveLinks are disjunct. But it is something to think
>>> about.
>> are you saying that the renderThread Image has a parent pointer to the
>> create thread TexObjChunk? Uhm, is there any code in the system that is
>> prepared to deal with pointers from one aspect copy into another?
>
> no, my assumption with the weak ptr was wrong so this conclusion was
> wrong to ;).
phew, cross aspect pointer sound scary to me ;)
>>> - If the createThread manages to change the image::parent field
>>> again after the sync the parent link in the rendering thread is
>>> lost. That is not really nice and has cropped up here and there
>>> (most famously in window) and something that needs a general
>>> solution.
>> hm, I may be missing something here and in that case I'm happy to change
>> my opinion on this, but from what I understand the rules for using
>> createLocal() are:
>> - an pointer to a createLocal() object may only be stored in a field of
>> an object that itself is createLocal().
>> - any pointer stored in a field of a createLocal() object must point to
>> a createLocal() object (this can be relaxed to only apply to children
>> fields).
>>
>> The latter is violated in SimpleTexturedMaterial as it puts the Image
>> pointer in to the createLocal() TexObjChunk.
>
> I don't see either a problem. The problem was that the image parent
> pointer field contained a NULL pointer. Or more general that the
> pointer to a createLocal object is not synced, so it is NULL in
> the target aspect.
ok, I think here is the core of the problem: you prefer much more
relaxed rules for where one may use createLocal() objects compared to
what I've outlined above. Do you propose any restrictions on them or do
you think it should be possible to use them essentially everywhere?
Wouldn't that imply every iteration over a pointer field would have to
be ready to deal with intermittent NULL entries?
Does it make sense to add this (easy to forget) requirement for
something that you said above "inside OpenSG itself they aren't of much
use" ? - These are not rhetoric questions, I'm curious how you see it
and what the tradeoffs are/
> So my take is that this allows to violate our 'no NULL ptr in field'
> assumptions and that this is the thing that should be fixed.
> I'm currently looking into this.
ok, thanks.
Cheers,
Carsten
------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
Opensg-core mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensg-core
