[Dune] Returning Geometries As Objects

[Steffen Müthing]

Hi all,

Am 25.01.2012 um 09:49 schrieb Martin Nolte:

> Dear all,
> 
> as announced in December, there is now an implementation returning the geometry as an object and all grid implementations in dune-grid have been adapted accordingly. Therefore, I think time has come for discussion of the outcome.
> 

> 
> As already mentioned in the announcement of the transition, there is also a subtle impact on user code. Geometry references may no longer be stored outside the scope calling the geometry method. For example:
> 
> const Geometry *pgeo = 0;
> {
>  const Geometry &geo = entity.geometry();
>  pgeo = &geo;
> }
> 
> worked with the current interface but will break after the transition. Notice that the compiler will now warn in such a situation and the resulting code will probably segfault. The good news is that the segfault is extremely likely (since, e.g., pgeo now points onto the execution stack). However, such code will be rare and probably only found in the higher discretization modules, such as dune-fem, dune-fufem, or dune-pdelab.


I just realized that this problem can also come up when the reference is a
class member, as might happen in a caching structure of a discretization
module (like the first argument passed to the local operator in PDELab),
and any code with a cache object like this:

template<typename GV>
class Cache
{

 Cache(const Entity& e)
   : geometry(e.geometry())
 {}

 const Geometry& geometry;

};

will probably break, as the compiler will destroy the temporary returned by
the call to Entity::geometry() as soon as the constructor exits (cf. section
12.2.5 of the standard). As Martin pointed out, the referenced address is
on the stack and will probably be overwritten pretty quickly, but I'm not sure
whether that will always cause a direct segfault or instead taunt us with subtle
and hard-to-track bugs. And unfortunately, we don't get a compiler warning
in this case (at least not with plain -Wall).

In PDELab we do not run into this problem right now because we only cache
the entity and not the geometry, but I don't know about the other assembler
frameworks out there.

The second area where this issue might show up is in metagrids that simply
reuse the underlying geometry - in that case, the Geomety wrapper needs to
somehow store the geometry, and as it can only do that by reference or using
a pointer, all metagrid maintainers should probably check whether their grid
works using Martin's branch (I admit, I haven't done any checking on
MultiDomainGrid yet, but I am pretty sure that my grid will have to be modified
to work with this change - will do some testing tomorrow).

Even if we decide that the issue is not a real problem for the geometries, I'm afraid
that it might cause a lot of major breakage in user code once we start the conversion
to value-typed Entities (I do hope most everyone caches those, or all of our courses
have been in vain ;-) ).

As I said, I don't know how dangerous this is in reality (especially for the geometries),
but I thought I'd better bring it up...


Steffen

PS: I attached a very simple example illustrating the issue.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: constref.cc
Type: application/octet-stream
Size: 1106 bytes
Desc: not available
URL: <https://lists.dune-project.org/pipermail/dune/attachments/20120208/360537ac/attachment.obj>
-------------- next part --------------



Steffen Müthing
Universität Stuttgart
Institut für Parallele und Verteilte Systeme
Universitätsstr. 38
70569 Stuttgart
Tel: +49 711 685 88429
Fax: +49 711 685 88340
Email: steffen.muething at ipvs.uni-stuttgart.de