Re: Thread-safety and STL
From: Doug Harrison [MVP] (dsh_at_mvps.org)
Date: 09/09/04
- Next message: WRH: "Re: HtmlHelp API"
- Previous message: Priyesh: "Re: CRT new exception"
- In reply to: Joe: "Re: Thread-safety and STL"
- Next in thread: Joe: "Re: Thread-safety and STL"
- Reply: Joe: "Re: Thread-safety and STL"
- Messages sorted by: [ date ] [ thread ]
Date: Thu, 09 Sep 2004 12:49:14 -0500
Joe wrote:
>"Doug Harrison [MVP]" <dsh@mvps.org> wrote in message
>news:b1v0k01amiap6ifafm6id4nuge37gv89t1@4ax.com...
>> Joe wrote:
>>
>
><snip>
>> >
>> >std::vector<BSTR> *pvec = new std::vector<BSTR>;
>>
>> Do you really need to allocate pvec dynamically? C++ is not C#.
>
>Maybe not, I pass the vector between functions so it seemed easiest. But is
>it a problem as long as I remember to delete?
Raw pointers such as pvec introduce exception safety issues, for who will
delete pvec if an exception is thrown? Smart pointers are the answer to
that, but you shouldn't use a pointer unless there's a good reason to. If
the vector is a member of a class, it typically shouldn't be a pointer. If
it's a local variable, which isn't intended to outlive the scope in which
it's declared, it should be a normal auto variable. Major reasons to
dynamically allocate objects include:
1. Extending lifetime beyond declarative region.
2. Allocating different types of derived objects to bind to a common base
class pointer.
3. Deferring creation of an object until it's required.
4. Creating arrays whose size isn't known until runtime (but vector or other
array classes are preferable).
"Passing the vector between functions" is probably not a good reason. If the
functions require the vector, you should use reference parameters. If they
don't, you should use a pointer parameter (which allows you to pass NULL),
and callers should take the address of the vector to pass them to the
function.
>> If another thread can access the changing data, you need to protect it
>with
>> a mutex of some sort. In MFC, you would probably use CCriticalSection.
>
>Right, but in actual fact in my app each thread creates its own vector for
>its own exclusive use, so I though I could get away without locking the
>vector.
In that case, you can, as vector itself has no internal data it shares
between threads. However, in VC6 and earlier, see this page for important
restrictions on <xtree> and <xstring>, which are used in the implementation
of the associative containers and std::string, respectively:
http://www.dinkumware.com/vc_fixes.html
>I thought I could just lock on the predicate class as I show below,
>but after the predicate class is created it is also deleted before remove_if
>is finished - ie the lock is released - I don't really understand why.
>
>
>class CriticalSectionLock
>{
>public:
>CriticalSectionLock()
>{
>InitializeCriticalSection(&m_CritSect);
>EnterCriticalSection(&m_CritSect);
>}
>~CriticalSectionLock()
>{
>LeaveCriticalSection(&m_CritSect);
>DeleteCriticalSection(&m_CritSect);
>}
>private:
>CRITICAL_SECTION m_CritSect;
>};
>class PredSSbstr
>{
>private:
>BSTR name_to_match;
>CriticalSectionLock plock;
>public:
>PredSSbstr(BSTR str): name_to_match(str)
>{
>}
>bool operator() (BSTR const &str)
>{int i;
>CEnBSTR bstrExt=str;
>if ((i=bstrExt.find(name_to_match))<0)
>return true;//return oppostite as this works with remove_if()
>else
>return false;
>}
>};
That's got a number of flaws:
1. Lock objects such as above should only ever be declared as local
variables, e.g.
void f()
{
Lock lock(mutex);
... access data shared between threads
}
2. You've written a dtor, but no copy ctor or assignment operator.
Generally, it's all or nothing, though "all" can (and frequently does)
include disabling copying, e.g. in a class X, declare but don't define
private members:
private:
// Copyguard
X(const X&);
void operator=(const X&);
To see the problem, note that predicate objects are passed around by value,
which means using the copy ctor. The default copy ctor does member-wise
copying, and this of course doesn't acquire the mutex. Your dtor releases
and destroys the CRITICAL_SECTION, and so you end up releasing and
destroying the CRITICAL_SECTION multiple times, once for each copy. That's
not good.
3. You don't need to be using a mutex at all, as you appear to use no data
shared between threads.
4. The CRITICAL_SECTION should not be a member of the lock class. The lock
class should store a pointer or reference to the thing it's locking. Only in
that way can multiple threads share the synchronization object, and it's
these objects they should share, not the locks.
5. The lock usage above serves no purpose whatsoever, since each predicate
object has its own CRITICAL_SECTION, and you're not sharing these objects
between threads.
>So as the above doesn't work, must I do something like:
>
>EnterCriticalSection(&m_CritSect);
>itrNewEnd = remove_if (pvec->begin( ),
>pvec->end( ),PredSSbstr(L"searchtext") );
>LeaveCriticalSection(&m_CritSect);
>
>each time that I wish to use the predicate PredSSbstr() ?
That's how you must perform _all_ access to an object shared between
threads. But since you're not sharing any objects between threads, you don't
need to do this. Note that the above is not exception-safe. Consider what
happens if an exception is thrown after you acquire the lock but before you
release it. Avoiding this problem is the reason you use a lock class, e.g.
// BLOCK
{
Lock lock(cs);
remove_if();
}
The lock is automatically released no matter how the block is exited,
whether it be falling off the end, early return statement, break, continue,
goto, or exception. (Sometimes, it's good to introduce a compound statement
as above to keep the lock for as short a time as possible. I always comment
as above to make it clear I didn't delete an if(), for(), etc and actually
intended to write a freestanding block.)
-- Doug Harrison Microsoft MVP - Visual C++
- Next message: WRH: "Re: HtmlHelp API"
- Previous message: Priyesh: "Re: CRT new exception"
- In reply to: Joe: "Re: Thread-safety and STL"
- Next in thread: Joe: "Re: Thread-safety and STL"
- Reply: Joe: "Re: Thread-safety and STL"
- Messages sorted by: [ date ] [ thread ]
Relevant Pages
|
