[geos-devel] Exception-safety (230KB)

Fri Apr 7 04:23:47 EDT 2006

I've made some tests with auto_ptr<> and I'm pretty satisfied with it.
Semantic for auto_ptr<> is that whenever it's copied ownership of
the contained pointer is transferred. Seems perfect for our
ownership-taking Geometry constructors. It's exception safe and
is provided by the standard template library.

We can keep the existing interface (would initialize the auto_ptr<>
by the given 'raw' pointers) plus add constructors taking auto_ptr<>.

Objections ?

--strk;

On Thu, Apr 06, 2006 at 03:39:56AM +0200, Mateusz Å?oskot wrote:
> Hi,
> 
> Recently, I reported some problems about possible memory leaks that can
> occur when Geometries construction is interrupted by exception.
> Until now, I reported bugs #86 and #89 related to LineString class and
> Point class. But I expect there are more similar places because current
> approach of constructor implementation is a kind of "common pattern"
> widely used in GEOS.
> 
> The problem applies only to situation when constructor throws an
> exception (as in LineString or Point class). In this situation
> destructor is never called and dynamically allocated resources, like
> LineString::points sequence, are not deallocated.
> Reference:
> http://www.parashift.com/c++-faq-lite/exceptions.html#faq-17.4
> 
> So, I'd like to provoke small discussion about how to achieve
> exception-safety in geos. IMHO, Exception-safety is pretty important
> step to stability.
> 
> I see two obvious ways to promise exception safety
> and avoid memory leaks (examples are a bit pseudocode):
> 
> 1. Find unsafe places and put "delete XXX;" before every "throw"
> instruction in constructors:
> 
> Foo::p_ - internal data
> 
> Foo::Foo(Bar* p) : p_(p)
> {
>    if (something)
>    {
>       delete p_; // For our safety
>       throw std::exception();
>    }
> }
> 
> 2. Second solution and, honestly, my favourite is to use smart pointers.
> If Foo::p_ would be "a smart pointer" instead of raw pointer, then the
> code is simplier and developer is not required to remember about all
> those manual deallocations:
> 
> Foo::Foo(BarSmartPtr p) : p_(p)
> {
>    if (something)
>       throw std::exception();
> }
> 
> That's all.
> 
> I prepare small example program to test exceptions
> not-safety and safety in real code.
> I attached it to this post, file exception-safety-test.tar.gz
> Inside the package, you will find README.txt file with short
> instruction how to build it with VC++ and GCC.
> NOTE: You don't need a Boost. I included Boost's smart pointers to the
> package, so just read the README.txt and have fun :-)
> 
> There are two tests:
> 
> 1) test_ctor_exception (presents unsafety of raw pointers)
> 
> ------ test_ctor_exception - start ------
>    noleak: Foo::Foo()
> not-throwing: A::A()
> not-throwing: A::~A()
>    noleak: Foo::~Foo()
> 
>    bigleak: Foo::Foo()
> throwing: A::A()
> throwing: ctor says hello but dctor won't have any chance to say it!
> We are NOT cool!
> ------ test_ctor_exception - end ------
> 
> 
> PROBLEM: bigleak is not destroyed in the second case
> 
> 2) test_ctor_exception_smart_ptr (presents how to get rid of unsafety
> with shared_ptr).
> 
> ------ test_ctor_exception_smart_ptr - start ------
>    noleak1: Bar::Bar()
> not-throwing: B::B()
> not-throwing: B::~B()
>    noleak1: Bar::~Bar()
> 
>    noleak2: Bar::Bar()
> throwing: B::B()
> throwing: ctor says hello AND dctors of B and Bar also say hello!
>    noleak2: Bar::~Bar()
> We are VERY cool!
> ------ test_ctor_exception_smart_ptr - end ------
> 
> 
> 
> Conclusion
> 
> Discussions about smart pointers and Boost appear and disappear from
> time to time.
> I think it's worth to take some decision.
> I'd really recommend to start using "Smart Pointer" idioms.
> Smart Pointers does not mean we should use Boost's smart pointers, we
> can write our own simple smart pointers but the question is if it's
> worth of the effort.
> Boost's smart pointers are well tested and well working,
> so why not to use them?
> 
> I'd summary my opinion as follows:
> There should be no question if GEOS should use smart pointers or not,
> because smart pointers can bring invaluable help; the only question I
> see is which smart pointers implementation to use: Boost, our own, ...
> Next, smart pointers can help not only to fix the problems
> described in this post but much more.
> 
> NOTE: It is NOT required to build any Boost library to use smart
> pointers. As you can see in the example I attached, they are just
> simple headers.
> We don't have to force users to install complete Boost.
> We can extract every library we wish to use just as I did so with smart
> pointers in the attached example. There is a special tool called bcp.
> It is not perfect but works well. The only imperfection I know is that
> it extracts quite big amount of files (e.g. for smart pointer, 2.7MB).
> Doing it manually makes it possible to get much smaller package.
> But I think the disc space is not a problem today :-)
> 
> 
> One more Boost digression
> 
> Boost will be included in the C++ Library soon.
> Recently, shared_ptr became a part of C++ Library Technical Report 1:
> http://boost.org/libs/smart_ptr/shared_ptr.htm#Introduction
> 
> Cheers
> -- 
> Mateusz Åoskot
> http://mateusz.loskot.net

> _______________________________________________
> geos-devel mailing list
> geos-devel at geos.refractions.net
> http://geos.refractions.net/mailman/listinfo/geos-devel

-- 

 /"\    ASCII Ribbon Campaign
 \ /    Respect for low technology.
  X     Keep e-mail messages readable by any computer system.
 / \    Keep it ASCII. 