Re: [PATCH] Dynamic SSL Certificate Generation

On 11/11/10 23:38, Tsantilas Christos wrote:
> On 11/11/2010 03:11 AM, Amos Jeffries wrote:
>> On Wed, 10 Nov 2010 17:05:16 +0200, Tsantilas Christos
>>>
>>> * Introduce the TidyPointer class similar to std::auto_ptr, which
>>> implements a pointer that deletes the object it points to when the
>>> pointer's owner or context is gone. It is designed to avoid memory
>>> leaks in the presence of exceptions and processing short cuts.
>>
>> So whats different about this new pointer type that the existing
>> std:auto_ptr, RefCount::Pointer and CbcPointer are all insufficient?
>
> The TidyPointer is completely different than RefCount and CbcPointers.
>
> It is similar to the std::auto_ptr but allow developer to define the
> deallocator method for the object it points to.
> Also does not implement the "=" operator and prevents developers from
> using the copy constructor.
>
> It can be usefull when you have code as the following:
> -----------------------
> A *a= newA();
> if (!a)
> return NULL;
> B *b=newB();
> if (!b) {
> freeA(a);
> return NULL;
> }
> C *c=newC();
> if (!c) {
> freeA(a);
> freeB(b);
> return NULL;
> }
>
> If(problem){
> freeA(a);
> freeB(b);
> freeC(c);
> return NULL;
> }
> return C;
>
> Using TidyPointer the above can be written as:
>
> TidyPointer<A> a(newA(),freeA);
> if (!a)
> return NULL;
> TidyPointer<B> b(newB(),freeB);
> if (!b)
> return NULL;
> TidyPointer<C> c(newC(), freeC);
> if (!c)
> return NULL;
>
> if (problem)
> return NULL;
> return C.release();
>
>
> Can help you to avoid memory leeks when you have the following case:
>
> A *a= newA();
> DoSomeWorkWhichMayThrow(a);
> freeA(a);
>
> In the above case if the DoSomeWorkWhichMayThrow() throws the a object
> will never be released.
>
> The above can be written:
>
> TidyPointer<A> a(newA(),freeA);
> DoSomeWorkWhichMayThrow(a.get());
>
>>
>>> From what I've heard of the SSL problems they are all cause by it not
>> using RefCount on the contexts themselves.
>>
>
> Maybe RefCount is usefull for the SSL_CTX objects. But I do not think it
> is required.
>
> In most cases using the openSSL library you have code like the following:
>
> a=NewA();
> b=newB(a);
> if (!b) {
> freeA(a);
> }
> else {
> //Just forget the a
> }
>
> I these cases the TidyPointer is very useful.
>
> Regards,
> Christos
>

Yes. Thank you for that.

So it is a special case of RefCount() with enforced maximum count of 1.
Using a specified C-function fro freeing instead of the objects own
destructor. So a plus for C objects without destructors.

Like all the smart pointers TidyPointer will delete it's data if it ever
dies. Yet it provides a get() method which allows duplication of the raw
pointer. This permits the following case:

   TidyPointer<a> A(a, free-a);
   void *B = A->get();
   A = NULL;
   if (!B) ...

Use of RefCount is preferrable here since B can be recognized.

Over in the dynamic generation code this nasty usage case is implemented
by the http_port contexts and again in the context "storage".

Specifically on the http_port... if you recall the history of bug
http://bugs.squid-cache.org/show_bug.cgi?id=2586 the destructor of
http_port globals are perfectly capable of cleanly erasing the member
contexts. We had to leave the SSL context leaking until they could be
*RefCounted* (with a full 1->N count) because active connections held
raw-pointers to it long after the "owner" port was closed.
   Using a TidyPointers there, with get() method in httpsAccept() to
attach to SSL* children is exactly identical logic to our previous
attempt at making the http_port destructor cleanup.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.9
   Beta testers wanted for 3.2.0.3