🚀 go-pugleaf

Mike Collins wrote:

>I have a thread with a getter and setter to provide access to a private 
>variable.  I use a CRITICAL_SECTION to ensure thead-safety but i wasn't sure 
>about whether or not to use a try.. __finally to ensure that the CS always 
>gets released...

Why would you need a _finally?
Can the variable-access throw?

>try... __finally to ensure that a critical section is always released?

If so, how about a CS class of some sort?
(May be typing/logic/C++ errors)

ret_type Getter()
{	CS	cs(MyThreadCS);	// constructor enters critical section
	return	this.value;
}			// destructor releases critical section
	
class CS
{	LPCRITICAL_SECTION lpCriticalSection;
	CS(LPCRITICAL_SECTION CriticalSection)
		lpCriticalSection = CriticalSection
	{	EnterCriticalSection( lpCriticalSection ); }
	~CS(){	LeaveCriticalSection( lpCriticalSection ); }
}

"Mike Collins" <its@TheBottomOfThePost> wrote:

>Hay all, just a quicky, just wondering if anyone had any thoughts on using 
>try... __finally to ensure that a critical section is always released?

Don't do it.

Make an object that has a destructor. In the destructor, put the code to
release the critical section. Now just put the object on the stack -
when the current scope is exited, the critical section *WILL* be
released.

This is a fundamental idea in C++, known as 'Resource Allocation Is
Initialisation' or RAII. You make sure that any resource that must be
allocated and released is owned by an object that, if it still has it
when it gets to destruct, releases it. std::string works this way.
std::fstream works this way. std::vector works this way.

Using try/__finally is not the C++ way, it's the way of Java and other
languages that can't use deterministic destruction.

Alan Bellingham
-- 
Team Browns
<url:http://www.borland.com/newsgroups/> Borland newsgroup descriptions
<url:http://www.borland.com/newsgroups/netiquette.html>      netiquette

"Mike Collins" <its@TheBottomOfThePost> wrote in message 
news:478a96e3@newsgroups.borland.com...
> Hay all, just a quicky, just wondering if anyone had any thoughts on using 
> try... __finally to ensure that a critical section is always released?
>
> I have a thread with a getter and setter to provide access to a private 
> variable.  I use a CRITICAL_SECTION to ensure thead-safety but i wasn't 
> sure about whether or not to use a try.. __finally to ensure that the CS 
> always gets released...

Besides the fact that try/finally is non-standard, it has also had bugs in 
the past, so I would not trust it *at all*.  In the vein of using RAII as 
Bob and Alan have suggested, my personal preference is to use ScopeGuard. 
ScopeGuard lets you create unnamed, stack-based objects that will invoke any 
function or method upon exiting the current, local scope.  For example:

#include "ScopeGuard.h"

void DoSomethingWithCritSect( TCriticalSection *pCritSect )
{
    pCritSect->Acquire();
    ON_BLOCK_EXIT_OBJ( *pCritSect, &TCriticalSection::Release );

    // code relying on the critical section goes here

} // <== pCritSect->Release is invoked here, regardless of how scope was 
exited

An article discussing the implementation and use of ScopeGuard was written 
by Andrei Alexandrescu and Petru Marginean and was published in CUJ in 
December 2000.  You can read it here:  http://www.ddj.com/dept/cpp/184403758

and get the source here:

ftp://66.77.27.238/sourcecode/cuj/2000/cujdec2000.zip

- Dennis 

Mike Collins wrote:

> Hay all, just a quicky, just wondering if anyone had any thoughts on
> using try... __finally to ensure that a critical section is always
> released?
> 
> I have a thread with a getter and setter to provide access to a
> private variable.  I use a CRITICAL_SECTION to ensure thead-safety
> but i wasn't sure about whether or not to use a try.. __finally to
> ensure that the CS always gets released...

Yes, you can write:

int MyClassGetProp(void)
{
 int res;
 EnterCtiticalSection(&CS);
 try
 {
  /*some code with possible exception*/
  ...
  res=/*some result*/;
 }
 __finally
 {
  LeaveCriticalSection(&CS); // will be executed in any case
 }
 return res;
}

Due to some problems in compiler you should
1) avoid direct ("goto") or indirect ("return") leaving try section.
'Return' is not prohibited but generates a lot of binary code
(intermediate variable for result is much better);
2) don't throw exceptions in __finally section (in theory it should
generate abort but it doesn't);
3) avoid complicated exception handling in __finally (see QC30802)
4) NEVER use 'goto' or 'return' for leaving __finally section.
5) use CB2007 where some problems were fixed (like QC21708)

-- 
Alex

P.S. Please, don't beat me for the post: I know that try/__finally
isn't pure cpp way :-)

"Mike Collins" <its@TheBottomOfThePost> wrote in message 
news:478a96e3@newsgroups.borland.com...

> Hay all, just a quicky, just wondering if anyone had any
> thoughts on using try... __finally to ensure that a critical section
> is always released?

That wll work fine, as long as you do not have any 'return' statements 
inside the 'try' section, as 'return' is an immediate exit that does not 
allow '__finally' to run correctly.

A better approach is to use an RAII object instead, where the constructor 
enters the critical section and the destructor leaves it.  That way, the CS 
is always guaranteed to be unlocked when the object goes out of scope, and 
you don't need to use a try...__finally block at all.

> I have a thread with a getter and setter to provide access to a private 
> variable.  I use a CRITICAL_SECTION to ensure thead-safety but i
> wasn't sure about whether or not to use a try.. __finally to ensure that
> the CS always gets released...

Either approach will work fine in that situation, ie:

    --- try..finally ---

    int MyThread::GetSomething()
    {
        int tmp;
        EnterCriticalSection(&fMyCS);
        try
        {
            tmp = fSomething;
        }
        __finally
        {
            LeaveCriticalSection(&fMyCS);
        }
        return tmp;
    }

    void MyThread::SetSomething(int Value)
    {
        EnterCriticalSection(&fMyCS);
        try
        {
            // assign Something...
        }
        __finally
        {
            LeaveCriticalSection(&fMyCS);
        }
    }


    --- RAII ---

    class TCSGuard
    {
    private:
        LPCRITICAL_SECTION fCS;
    public:
        TCSGuard(LPCRITICAL_SECTION aCS)
            : fCS(aCS)
        {
            if( fCS ) EnterCriticalSection(fCS);
        }

        ~TCSGuard()
        {
            if( fCS ) LeaveCriticalSection(fCS);
        }
    };


    int MyThread::GetSomething()
    {
        TCSGuard g(&fMyCS);
        return Something;
    }

    void MyThread::SetSomething(int Value)
    {
        TCSGuard g(&fMyCS);
        // assign Something...
    }


Gambit 

"Dennis Jones" <nospam@nospam.com> wrote in message 
news:478b0076$1@newsgroups.borland.com...

> An article discussing the implementation and use of ScopeGuard was written
> by Andrei Alexandrescu and Petru Marginean and was published in CUJ in
> December 2000.

Nice article.

I had always been a little disappointed with pure RAII techniques since it 
seemed silly to have to make full class every time a new use situation 
arose.  I stuck with it though (and occassionally slipped in a well 
documented try...__finally block).

After testing ScopeGuard I might make the switch.  It even works with 
closures.

I have one itty bitty problem with using this approach.  I haven't looked 
closely enough yet to see why it's happening.  If full warnings are enabled 
(specifically -w-aus) and I use a standard RAII class (like std::auto_ptr or 
something I've made) then the following compiles in Builder without warning 
(in BCB6 and CB2007):

  class TMyAuto
  {
    private:
      char* Ptr;

    public:
      TMyAuto(char* APtr) : ptr(APtr) {}
      ~TMyAuto() { delete Ptr; }
  };

  char* c = new char;
  TMyAuto auto_c(c);

  // This also works with no warning
  // std::auto_ptr<char> auto_c(c);

However, if I use ScopeGuard I receive a W8004 warning:

  void DeleteIt(char* Ptr)
  {
    delete Ptr;
  }

  char* c = new char;
  ON_BLOCK_EXIT(&DeleteIt, c);

  // This also causes a W8004 warning
  // ScopeGuard auto_c(MakeGuard(&DeleteIt, c));

  W8004 'scopeGuard321' is assigned a value that is never used

As I said above I haven't looked too much into this.  I don't know if 
Builder should be mentioning the warning in all cases or if it follows some 
rule that ScopeGuard isn't triggering.  In any case, I still like ScopeGuard 
and will investigate the warning issue at a later date.

Clayton 

"Clayton Arends" <nospam_claytonarends@hotmail.com> wrote in message 
news:478bac9a$1@newsgroups.borland.com...

> I have one itty bitty problem with using this approach.  I haven't looked 
> closely enough yet to see why it's happening.

Well, I dug a little deeper.  If I use ScopeGuardImpl1 (for example) 
directly rather than using the MakeGuard() function then I can avoid the 
warning:

  ScopeGuardImpl1<void (*)(char*), char*> auto_a =
    ScopeGuardImpl1<void (*)(char*), char*>::MakeGuard(&DeleteIt, a);

The warning is most likely caused by the trick ScopeGuard uses to 
circumnavigate the need for a virtual destructor.  Also Builder is probably 
not correctly hiding the warning like it does for the standard RAII object 
situations.  I don't know enough about the C++ rules to begin to author a QC 
feature request for this problem but I have a feeling my best course of 
action is to either live with the warning or disable it around the affected 
code.

Clayton

"Clayton Arends" <nospam_claytonarends@hotmail.com> wrote in message 
news:478bba5f$1@newsgroups.borland.com...
> "Clayton Arends" <nospam_claytonarends@hotmail.com> wrote in message 
> news:478bac9a$1@newsgroups.borland.com...
>
>> I have one itty bitty problem with using this approach.  I haven't looked 
>> closely enough yet to see why it's happening.

I believe the warning is correct.  It is the result of scopegaurd creating 
an RAII object whose destructor is invoked when it goes out of scope, but is 
not used otherwise.  It is no different, say, than:

void test()
{
    int i=1;  // assigned a value, but never used -- may generate W8004
}


> Well, I dug a little deeper.  If I use ScopeGuardImpl1 (for example) 
> directly rather than using the MakeGuard() function then I can avoid the 
> warning:
>
>  ScopeGuardImpl1<void (*)(char*), char*> auto_a =
>    ScopeGuardImpl1<void (*)(char*), char*>::MakeGuard(&DeleteIt, a);

That's interesting, because I would actually *expect* the same warning 
there, because you are assigning a value to auto_a, but never using it.

[
Aside: the & (address of) operator is not needed:

    ScopeGuardImpl1<void (*)(char*), char*>::MakeGuard(DeleteIt, a);
]

Also, if you use MakeGuard() and then have to dismiss the guard later, that 
constitutes usage that would eliminate the warning.  For example:

void test()
{
    char* c = new char;
    ScopeGuard DeleteGuard = MakeGuard( DeleteIt, c );  // no warning 
because DeleteGuard gets used (via Dismiss()) later
    delete c;
    DeleteGuard.Dismiss();
}

Although using ScopeGuard is not perfectly clean with regard to warnings, 
that particular warning is, IMO, extremely minor, especially when compared 
to the benefit that ScopeGuard provides.  If you really dislike the warning, 
you can always use "pragma warn" to get rid of it:

#pragma warn -8004
void test()
{
    char* c = new char;
    ON_BLOCK_EXIT( DeleteIt, c );
}
#pragma warn .8004

- Dennis 

"Dennis Jones" <nospam@nospam.com> wrote in message 
news:478be3e4$1@newsgroups.borland.com...

> I believe the warning is correct.  It is the result of scopegaurd
> creating an RAII object whose destructor is invoked when it
> goes out of scope, but is not used otherwise.

Then the same warning should apply to any other RAII object that is created 
and destroyed without being accessed otherwise.


Gambit 

"Remy Lebeau (TeamB)" <no.spam@no.spam.com> wrote in message 
news:478bf841$1@newsgroups.borland.com...
>
> "Dennis Jones" <nospam@nospam.com> wrote in message 
> news:478be3e4$1@newsgroups.borland.com...
>
>> I believe the warning is correct.  It is the result of scopegaurd
>> creating an RAII object whose destructor is invoked when it
>> goes out of scope, but is not used otherwise.
>
> Then the same warning should apply to any other RAII object that is 
> created and destroyed without being accessed otherwise.

Right (it seems only logical).  What I don't understand, I guess, is why one 
would expect it NOT to generate a warning.

- Dennis 

Dennis Jones wrote:

>What I don't understand, I guess, is why one 
>would expect it NOT to generate a warning.

If the creation is a function, then that
negates the uselessness test.

"Bob Gonder" <notbg@notmindspring.invalid> wrote in message 
news:e45oo3tf9dfrtlm8udetauha7jp5s4glam@4ax.com...
> Dennis Jones wrote:
>
>>What I don't understand, I guess, is why one
>>would expect it NOT to generate a warning.
>
> If the creation is a function, then that
> negates the uselessness test.

Not sure what you are getting at here, Bob.  A ScopeGuard object is created 
and assigned to a variable; that variable is never used (not directly 
anyway) after the assignment, and that's what triggers the warning.  Using 
the variable (to Dismiss() the guard, for example) obviates the warning.

- Dennis 

"Dennis Jones" <nospam@nospam.com> wrote in message 
news:478be3e4$1@newsgroups.borland.com...

> [
> Aside: the & (address of) operator is not needed:
>
>    ScopeGuardImpl1<void (*)(char*), char*>::MakeGuard(DeleteIt, a);
> ]

That's personal preference.  I always prefer the address-of notation when 
grabbing the address of a function/method.  There's a pretty big thread 
discussion about this that I was involved in if you'd like to look for it. 
;-)

Clayton 

"Dennis Jones" <nospam@nospam.com> wrote in message 
news:478c17ad$2@newsgroups.borland.com...

> "Bob Gonder" <notbg@notmindspring.invalid> wrote in message 
> news:e45oo3tf9dfrtlm8udetauha7jp5s4glam@4ax.com...
>> If the creation is a function, then that
>> negates the uselessness test.
>
> Not sure what you are getting at here, Bob.

I think the uselessness test applies even if the class was created without a 
function.  The nature of classes is that they can do anything on 
construction and anything on destruction.  This simple fact means that the 
compiler cannot reliably know if the variable has no practical use.  When 
the compiler checks for use of native types such as int, float, or char it 
knows that if the variable is never used then it can display a warning to 
the user.  However, with classes the compiler doesn't really know anything. 
The unused variable could have some real use (like RAII classes) or it could 
be a useless variable (like the many string classes, TDateTime, etc).

Because of this I think the only logical choice a programmer has is to 
disable the warning (globally or where needed).

Clayton 

"Clayton Arends" <nospam_claytonarends@hotmail.com> wrote in message 
news:478c5842$1@newsgroups.borland.com...
> "Dennis Jones" <nospam@nospam.com> wrote in message 
> news:478be3e4$1@newsgroups.borland.com...
>
>> [
>> Aside: the & (address of) operator is not needed:
>>
>>    ScopeGuardImpl1<void (*)(char*), char*>::MakeGuard(DeleteIt, a);
>> ]
>
> That's personal preference.  I always prefer the address-of notation when 
> grabbing the address of a function/method.  There's a pretty big thread 
> discussion about this that I was involved in if you'd like to look for it. 
> ;-)

Thanks.  It seems there are a number of cases where it is required, and 
since it may also be redundant but without doing any harm, I may have to 
change my habit.  It's also more or less in line with my preference to be 
explicit in cases where there is potential for ambiguity.  Thanks for the 
nudge.

- Dennis 

"Clayton Arends" <nospam_claytonarends@hotmail.com> wrote in message 
news:478c5a53$1@newsgroups.borland.com...
> "Dennis Jones" <nospam@nospam.com> wrote in message 
> news:478c17ad$2@newsgroups.borland.com...
>
>> "Bob Gonder" <notbg@notmindspring.invalid> wrote in message 
>> news:e45oo3tf9dfrtlm8udetauha7jp5s4glam@4ax.com...
>>> If the creation is a function, then that
>>> negates the uselessness test.
>>
>> Not sure what you are getting at here, Bob.
>
> I think the uselessness test applies even if the class was created without 
> a function.  The nature of classes is that they can do anything on 
> construction and anything on destruction.  This simple fact means that the 
> compiler cannot reliably know if the variable has no practical use.

Okay, I think I see what you mean.  If the object is of some class with a 
destructor, then the object *will* be used, even if it is not explicitly 
used in the code.  So, if the compiler generates the warning sometimes but 
not others, then I agree that the compiler may be deficient in its 
evaluation of cases where the warning is appropriate (I don't think I would 
go so far as to call it a bug though).

- Dennis