new and delete from different threads

D

Dilip

I am aware that the C++ standard in its present form does not say
anything about threads, however I do have a relevant question.

I am working on Windows XP/VC++ 8.0.

Is there a problem new'ing a bunch of objects from one thread and
deleting them in another? I do something like:

struct GenericPointerDeleter
{
template<typename T>
void operator()(T* p)
{
if (ptr != 0)
{
delete ptr;
ptr = 0;
}
}
};

typedef vector<classofpointers*> vecptrs;
int main()
{
vecptrs myptrs;
CreateThread(...... (LPVOID)&myptrs........);
// use some platform specific API to wait until
threadcallbackroutine terminates
pseudo_wait_for_terminate(threadcallbackroutine);

for_each(myptrs.begin(), myptrs.end(), GenericPointerDeleter());
}

DWORD WINAPI threadcallbackroutine(LPVOID param)
{
vecptrs* my_ptrs = static_cast<vecptrs*>(param);
my_ptrs->push_back(new classofpointers());
return 0;
}

I have vastly simplified what is essentially happening in my
application...

Should I be careful about new'ing and deleting from the same thread?
 
B

BigBrian

Dilip said:
I am aware that the C++ standard in its present form does not say
anything about threads, however I do have a relevant question.

What makes your question on topic? I don't see anything other than a
question about threading, which is not on topic since this newsgroup
only covers standard C++.
I am working on Windows XP/VC++ 8.0.

Yes, but that's irrelevant on this newsgroup.
Should I be careful about new'ing and deleting from the same thread?

If you do new and delete in the same thread ( in a multi-threaded
application ), how is that any different than doing new and delete in a
single threaded application?
 
G

Gianni Mariani

Dilip wrote:
....
Is there a problem new'ing a bunch of objects from one thread and
deleting them in another? ...

There should be no problem doing that on Win32, Linux, MAC or many other
unicies I've worked on.

For some versions of malloc, it will be somewhat less efficient but
unless performance is a big issue, then you should have no worries.
 
H

Howard Hinnant

"Dilip said:
I am aware that the C++ standard in its present form does not say
anything about threads, however I do have a relevant question.

I am working on Windows XP/VC++ 8.0.

Is there a problem new'ing a bunch of objects from one thread and
deleting them in another? I do something like:

struct GenericPointerDeleter
{
template<typename T>
void operator()(T* p)
{
if (ptr != 0)
{
delete ptr;
ptr = 0;
}
}
};

typedef vector<classofpointers*> vecptrs;
int main()
{
vecptrs myptrs;
CreateThread(...... (LPVOID)&myptrs........);
// use some platform specific API to wait until
threadcallbackroutine terminates
pseudo_wait_for_terminate(threadcallbackroutine);

for_each(myptrs.begin(), myptrs.end(), GenericPointerDeleter());
}

DWORD WINAPI threadcallbackroutine(LPVOID param)
{
vecptrs* my_ptrs = static_cast<vecptrs*>(param);
my_ptrs->push_back(new classofpointers());
return 0;
}

I have vastly simplified what is essentially happening in my
application...

Should I be careful about new'ing and deleting from the same thread?

I don't have access to Windows, but I believe your code is fine. The
C++ committee is interested in C++0X have a multi-threaded memory model
which will probably be based on current practice today. And as far as I
know, current practice is that transferring memory ownership across
thread boundaries is fine.

-Howard
 
M

mlimber

Dilip said:
I am aware that the C++ standard in its present form does not say
anything about threads, however I do have a relevant question.

You could frame it in terms of Boost.Threads, since something along
those lines will likely be included in C++09. Then it would be on
topic. :)
I am working on Windows XP/VC++ 8.0.

Is there a problem new'ing a bunch of objects from one thread and
deleting them in another? I do something like:

struct GenericPointerDeleter
{
template<typename T>
void operator()(T* p)
{
if (ptr != 0)
{
delete ptr;
ptr = 0;
}
}
};

First, there's no need to check for null before deleting. Second,
zeroing the pointer is generally unnecessary, though it might be useful
if you reuse the pointer (doesn't look like you do below) but such
reuse is often considered bad practice. Lastly, you're duplicating
existing functionality. You could use std::tr1::shared_ptr (aka
boost::shared_ptr) for a smart pointer that works with standard
containers.
typedef vector<classofpointers*> vecptrs;

If you do this...

typedef vector said:
int main()
{
vecptrs myptrs;
CreateThread(...... (LPVOID)&myptrs........);
// use some platform specific API to wait until
threadcallbackroutine terminates
pseudo_wait_for_terminate(threadcallbackroutine);

for_each(myptrs.begin(), myptrs.end(), GenericPointerDeleter());

....then this is completely unnecessary. It is done automagically by the
destructors.
}

DWORD WINAPI threadcallbackroutine(LPVOID param)
{
vecptrs* my_ptrs = static_cast<vecptrs*>(param);
my_ptrs->push_back(new classofpointers());
return 0;
}

I have vastly simplified what is essentially happening in my
application...

Should I be careful about new'ing and deleting from the same thread?

That could be platform specific. <OT>But in your case it probably
doesn't matter.</OT>

Cheers! --M
 
D

Dilip

Thanks to everyone that replied! Much appreciated. Now I can safely
look for my out of memory errors elsewhere.

I have some comments inline.
Dilip wrote:
First, there's no need to check for null before deleting.

I know.. more like defensive coding. :)
Second,
zeroing the pointer is generally unnecessary, though it might be useful
if you reuse the pointer (doesn't look like you do below) but such
reuse is often considered bad practice. Lastly, you're duplicating
existing functionality. You could use std::tr1::shared_ptr (aka
boost::shared_ptr) for a smart pointer that works with standard
containers.

If you do this...



...then this is completely unnecessary. It is done automagically by the
destructors.

I would like nothing better than using Boost but management has a
policy on open-source projects and one of them involves being paranoid
about downloading them. :) NIH syndrome is very prevalent around here.
 
M

mlimber

Dilip said:
I know.. more like defensive coding. :)

The FAQ calls it "wrong":

http://www.parashift.com/c++-faq-lite/freestore-mgmt.html#faq-16.8
I would like nothing better than using Boost but management has a
policy on open-source projects and one of them involves being paranoid
about downloading them. :) NIH syndrome is very prevalent around here.

Do they let you use the standard library? You might try to make the
case that shared_ptr is part of the standard C++ library working
group's technical report -- (currently) non-normative extensions to the
C++ standard library that will likely be included in the next version
of the library -- not just a run-of-the-mill open-source library.

Cheers! --M
 
H

Howard Hinnant

"mlimber said:
You might try to make the
case that shared_ptr is part of the standard C++ library working
group's technical report -- (currently) non-normative extensions to the
C++ standard library that will likely be included in the next version
of the library

News flash: shared_ptr was voted into the C++0X working draft on April
7, 2006. :)

I'm not meaning to contradict you. It is still non-normative as you
say. Just thought this to be an important update.

-Howard
 
T

Tamas Demjen

Dilip said:
I would like nothing better than using Boost but management has a
policy on open-source projects and one of them involves being paranoid
about downloading them. :) NIH syndrome is very prevalent around here.

shared_ptr is a de-facto standard, and soon-to-be official standard. The
Boost library is not distributed under the GPL, it has a very strict
policy to keep the code freely usable in commercial code. The library
strives for a very high quality, and many of the contributors are among
the most respected members of the C++ community.

shared_ptr is probably the single most valuable part of Boost. It truly
increases the stability of your code, not only by automatically cleaning
up resources, but also by virtually eliminating accidental double
deletions and dereferencing dead or uninitialized pointers. Using
weak_ptr you can ensure that your weak references automatically expire
when the last copy of the owned object they point to goes out of scope.

I've written an introductory tutorial about shared_ptr, which should get
you started:

http://tweakbits.com/articles/sharedptr/index.html

Tom
 
E

Earl Purple

Tamas said:
Dilip wrote:
shared_ptr is probably the single most valuable part of Boost. It truly
increases the stability of your code, not only by automatically cleaning
up resources, but also by virtually eliminating accidental double
deletions and dereferencing dead or uninitialized pointers. Using
weak_ptr you can ensure that your weak references automatically expire
when the last copy of the owned object they point to goes out of scope.

Would you know whether or not shared_ptr will be thread-safe?
 
H

Howard Hinnant

"Earl Purple said:
Would you know whether or not shared_ptr will be thread-safe?

So far, threading hasn't been introduced into the working draft. But
there is a significant effort to get multithreading issues into either
C++0X and/or a technical report. And a known issue is the thread safety
characteristics of shared_ptr.

All implementations of shared_ptr that I'm aware of currently make
shared_ptr as safe as a void*. You can concurrently manipulate two
copies of a shared_ptr, even if they manage the same underlying
reference. You may not concurrently manipulate a single shared_ptr.
And you may not concurrently manipulate a single pointee, even if
accessed via separate shared_ptrs. I.e. the reference count is thread
safe and nothing more.

This behavior is most likely that which will be adopted.

-Howard
 
M

mlimber

Tamas said:
I've written an introductory tutorial about shared_ptr, which should get
you started:

http://tweakbits.com/articles/sharedptr/index.html

Here's a section from the tutorial with my comments inline:
What Is Wrong With The Standard Auto Pointer?

I dispute your title. auto_ptr is a simple pointer for simple needs. As
Stroustrup says, it "isn't a general smart pointer. However, it
provides the service for which it was designed -- exception safety for
automatic pointers -- with essentially no overhead" (_TC++PL_, 3rd ed.,
sec. 14.4.2). IOW, it is not "wrong"; it's just different. Indeed, the
Boost documentation for their smart pointers says, "These templates are
designed to complement [not replace] the std::auto_ptr template."

In fact, I find it very useful for communicating transfer of ownership
without involving shared_ptr when the latter would add ambiguity and
unnecessary overhead. Here are two common uses:

class A { /*...*/ };

std::auto_ptr<A> CreateA()
{
return std::auto_ptr<A>( new A );
}

class B
{
boost::scoped_ptr<A> a_;
public:
B( std::auto_ptr<A> a ) : a_( a ) {}
// ...
};

The CreateA() function signature makes clear (and tries to enforce!)
that the user is responsible for deleting that pointer that is
returned, and if you happen to want to use the object in a shared_ptr,
conveniently there is a shared_ptr constructor for just that purpose.
Hence, the following line would work fine while still using the minimal
(i.e. zero-overhead) tool for the job since other users may have
different needs:

boost::shared_ptr<A> pa( CreateA() );

In the case of class B, the use of auto_ptr makes clear that B is
assuming ownership of the A object passed to it. It could use
shared_ptr (but not scoped_ptr!), but that would be less clear if the
desired behavior of B is that it solely own that A instance. (I use
scoped_ptr as the member instead of another auto_ptr because the former
doesn't allow any copying, so the implicitly generated copy constructor
and assignment operator won't give me any hidden problems.)
The std::auto_ptr has a tremendous disadvantage: It can not be copied
without destruction. When you need to make a copy of an auto pointer,
the original instance is destroyed. This means you may only have a
single copy of the object at any time.

First, this can be an advantage -- it just depends on the situation.
Second, the original instance is not "destroyed" (i.e. deleted).
Rather, the second instance of auto_ptr assumes ownership of the object
instance, and the first one no longer refers to it. Perhaps a better
word is "invalidated."
This also means that auto_ptr can not be
used with standard containers, such as vector, deque, list, set, and map.
In fact, it can hardly be used in any class that relies on copy construction.

True, and that is also sometimes desirable. scoped_ptr fits the same
description, but it is also often useful.
Furthermore, auto_ptr is not safe, because nothing prevents you from doing
a copy accidentally. And if you do so, you destroy the original copy.

True (if you replace "destroy the original copy" with "invalidate the
original auto_ptr"), but if you need to prevent accidental copying, you
should be using scoped_ptr. When the TR1/Boost smart pointers are
available, auto_ptr is most often used purely for *transferring*
ownership, not exception safety, ownership by a class, or copying
objects around. It could be better named, of course, as this bit of
history from the Boost docs relates:

"Greg Colvin proposed to the C++ Standards Committee classes named
auto_ptr and counted_ptr which were very similar to what we now call
scoped_ptr and shared_ptr. [Col-94] In one of the very few cases where
the Library Working Group's recommendations were not followed by the
full committee, counted_ptr was rejected and surprising
transfer-of-ownership semantics were added to auto_ptr."

As I've argued above, these transfer-of-ownership semantics are not
innately evil. They just must be used deliberately. When they aren't
desirable, a different smart pointer should be used instead.
Also,
some less standard compliant C++ compilers let you store forward declared
objects in an auto_ptr, and use that without ever including the full definition of
the class. This always results in a memory leak.

No. First of all, it's okay to delete such a class if the destructor is
"trivial", and second, deleting a declared-but-undefined class instance
with a non-trivial destructor results in undefined behavior, which
could be but is not necessarily a memory leak. That is actually a
"problem" with the language, not just some compilers, and Boost uses an
"ingenious hack" (boost::checked_delete) to prevent the unintentional
deletion of a declared-but-not-defined class instance. It could be
added to auto_ptr, too, without breaking any existing, working code
(i.e. code that doesn't already result in undefined behavior), but I
don't know if that's in the plans.

Cheers! --M
 
J

Jeremy Jurksztowicz

Last time I tuned in to the conversation, the 'consensus' was that
thread safety is built into the reference counting scheme, whether you
need it or not! Boost's shared pointer uses a macro to globally enable
and disable thread safety. I argued vehemently that a defaulted
template paramater would be preferable for those of us who need
different shared pointer thread safety at different program points, but
alas... I still can't wrap my head around the rationale for it's
current incarnation.

This is based on an old conversation with Peter Dimov, so it may be out
of date, but my boost 1.32 system uses the macro approach.

--Jeremy Jurksztowicz
 
T

Tamas Demjen

mlimber wrote:

Thanks for the feedback, I appreciate it.
No. First of all, it's okay to delete such a class if the destructor is
"trivial", and second, deleting a declared-but-undefined class instance
with a non-trivial destructor results in undefined behavior, which
could be but is not necessarily a memory leak. That is actually a
"problem" with the language, not just some compilers, and Boost uses an
"ingenious hack" (boost::checked_delete) to prevent the unintentional
deletion of a declared-but-not-defined class instance.

I should probably rephrase my article and explain it better. Many
compilers show an error message in that case, but I know Borland
C++Builder doesn't. If you try to delete a forward declared object from
a template (such as auto_ptr), it's very well possible that you don't
get any warning or error. If that object happens to have a non-default
destructor, it's not going to be called:

// in .h
class ForwardDeclared;

class Test
{
public:
Test();
private:
std::auto_ptr<ForwardDeclared> p;
};

// in .cpp:
#include "ForwardDeclared.h"

Test::Test() : p(new ForwardDeclared) { }

I know that this is a language issue, and I don't expect the compiler to
solve this problem. All I expect is a reliable error message. I agree,
checked_delete is an ingenious solution to ensure that an error message
is produced with every compiler. Since one of the compilers I use can't
produce an error message with auto_ptr, I don't feel safe using auto_ptr
as a member variable (at least not until my STL implementation adds
checked_delete into it).

Tom
 
M

mlimber

Tamas said:
mlimber wrote:

Thanks for the feedback, I appreciate it.


I should probably rephrase my article and explain it better. Many
compilers show an error message in that case, but I know Borland
C++Builder doesn't.

Just curious: Is it an error or a warning?
If you try to delete a forward declared object from
a template (such as auto_ptr), it's very well possible that you don't
get any warning or error. If that object happens to have a non-default
destructor, it's not going to be called:

To nitpick one more time: it's actually a "non-trivial" destructor not
a "non-default" one. The former is "Standardese for saying that the
class, one or more of its direct bases, or one or more if its
non-static data members has a user-defined destructor" (B. Karlsson,
_Beyond the C++ Standard Library: An Introduction to Boost_, p. 84).
Deleting an incomplete type also result in undefined behavior if the
class in question overloads the delete operator.
// in .h
class ForwardDeclared;

class Test
{
public:
Test();
private:
std::auto_ptr<ForwardDeclared> p;
};

// in .cpp:
#include "ForwardDeclared.h"

Test::Test() : p(new ForwardDeclared) { }

I know that this is a language issue, and I don't expect the compiler to
solve this problem. All I expect is a reliable error message. I agree,
checked_delete is an ingenious solution to ensure that an error message
is produced with every compiler.

Boost's rationale for always using checked_delete in these
circumstances is that "Some compilers issue a warning when an
incomplete type is deleted, but unfortunately, not all do, and
programmers sometimes ignore or disable warnings."
Since one of the compilers I use can't
produce an error message with auto_ptr, I don't feel safe using auto_ptr
as a member variable (at least not until my STL implementation adds
checked_delete into it).

As per my previous post, I agree that one should generally not use
auto_ptr as a class member; scoped_ptr is almost always the right
choice when one is tempted it use it as such.

Cheers! --M
 
T

Tamas Demjen

mlimber said:
Just curious: Is it an error or a warning?

Just verified it, it's a warning. In VC++ 2005, it's

warning C4150: deletion of pointer to incomplete type 'ForwardDeclared';
no destructor called
To nitpick one more time: it's actually a "non-trivial" destructor not
a "non-default" one. The former is "Standardese for saying that the
class, one or more of its direct bases, or one or more if its
non-static data members has a user-defined destructor" (B. Karlsson,
_Beyond the C++ Standard Library: An Introduction to Boost_, p. 84).
Deleting an incomplete type also result in undefined behavior if the
class in question overloads the delete operator.

Yes, that definition is a lot more precise. It's time to buy that book.
At the time I wrote my article, I wasn't able to find any practical
information about shared_ptr, so I decided to document my knowledge and
make it available to the others. You could say that the situation has
changed, and the book you mentioned is most likely more comprehensive
and exact than my little write-up.
Boost's rationale for always using checked_delete in these
circumstances is that "Some compilers issue a warning when an
incomplete type is deleted, but unfortunately, not all do, and
programmers sometimes ignore or disable warnings."

That's exactly the case. A warning would be OK with me, but again, some
compilers are completely silent about it.

Tom
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,599
Members
45,175
Latest member
Vinay Kumar_ Nevatia
Top