You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@thrift.apache.org by Vinicius Tinti <vi...@gmail.com> on 2009/11/20 00:46:35 UTC

Confusing code in PosixThread.start() [c++]

Hello,

I detect some memory leak in my application using valgrind.

The leak happen with PosixThread, after call PosixThread.start() the
reference count of shared_ptr increases by one. When the thread
loses it scope the count reference is with 2. So, there are 2 smart pointers
using the thread. One is mine, and the other?

I found this really strange code in PosixThread.cpp

PosixThread::start()

...
    // Create reference
    shared_ptr<PthreadThread>* selfRef = new shared_ptr<PthreadThread>();
    *selfRef = self_.lock();
...

So, it just creates a local scope smart pointer the let it leak, so, the
reference count will not be decreased letting my thread leaking.

Can some one explain me that?

Sorry by my poor english.

Thanks.

-- 
Vinicius Tinti -> viniciustinti@gmail.com, tinti@comp.eng.br,
vtintipo@gmail.com

"Here Goes Nothing"
"Beware of bugs in the above code; I have only proved it correct, not tried
it."
"Only those who attempt the absurd can achieve the impossible."
"Never underestimate the power of very stupid people in large groups."
"Simplicity is the ultimate sophistication"
"Beware, I know Ruby-FU"

Re: Confusing code in PosixThread.start() [c++]

Posted by Vinicius Tinti <vi...@gmail.com>.
Hi,

Thanks for your answer. I found an error in my code. By default, pthread
creates joinable
threads, but PosixThreadFactory don't. So, as commented in the source code,
a fail in
thread.join() (in this case because thread is detached) leads to a memory
leak.

There wasn't memory leak was mine bad usage of the library. See the short
explanation below:

int port = 9090;
shared_ptr<MyApplicationHandler> handler(new MyApplicationHandler());
shared_ptr<TProcessor> processor(new MyApplicationProcessor(handler));
shared_ptr<TServerTransport> serverTransport(new TServerSocket(port));
shared_ptr<TTransportFactory> transportFactory(new
TBufferedTransportFactory());
shared_ptr<TProtocolFactory> protocolFactory(new TBinaryProtocolFactory());

boost::shared_ptr<TThreadedServer> server(
    new TThreadedServer(processor, serverTransport, transportFactory,
protocolFactory));

/* WRONG
PosixThreadFactory thread_factory; // any_thread.join() will fail and leads
memory leak
*/

// RIGHT
PosixThreadFactory thread_factory(PosixThreadFactory::ROUND_ROBIN,
                                  PosixThreadFactory::NORMAL,
                                  1,
                                  false); // Detached
// now any_thread.join() should work

boost::shared_ptr<Thread> server_thread = thread_factory.newThread(server);

server_thread->start();

getchar();

server->stop();
server_thread->join();

Thanks again and sorry by inconvenience of many emails in mail list ( I
thought that my registration
had been refused then remade it and sent more emails ) and arguing that
Thrift has a memory leak.

To avoid this kind of error what do you think of put in a C++ FAQ "Why my
thread join is failing?" or
"Why my threads are apparently leaking?".

And how about put in Thrift default C++ generation code the code above. In
my opinion a lot of
applications will need to start or stop a server without blocking their
execution.

Sorry and thanks,
Vinicius.

On Tue, Nov 24, 2009 at 5:39 PM, David Reiss <dr...@facebook.com> wrote:

> The author of that code had this to say:
>
> A Thread has a weak reference to itself and a reference (shared_ptr)
> to it's Runnable object
>
> A Runnable has a weak reference to the Thread object.
>
> If a thread creator gives up it's reference to Runnable immediately
> after creating the thread, this guarantees that the Runnable WILL NOT
> be GC'ed.
> If a thread creator gives up both its Runnable and Thread before
> invoking Thread.start, this guarantees that both Thread and Runnable
> WILL be GC'ed
>
> In PthreadThread::start the weak ref is converted into a heap-based
> regular ref so that the thread doesn't get GC'ed before
> PthreadThread:threadMain is entered.  In threadMain, the heap-based
> regular ref is replaced by a stack-based ref.
> Doing this means that the Thread and Runnable objects WILL NOT get
> GCed until threadMain exits, even if all other references have been
> given up.
>
> So the answer to Vinicius' question is the 2 references are for 1) the
> creator thread and 2) the thread created by  PosixThread::start.  This
> is not a leak and works as designed.  I'm not clear why this would
> show up as a leak in valgrind unless he is not doing a clean exit.
>
>
> Vinicius Tinti wrote:
> > Hello,
> >
> > I detect some memory leak in my application using valgrind.
> >
> > The leak happen with PosixThread, after call PosixThread.start() the
> > reference count of shared_ptr increases by one. When the thread
> > loses it scope the count reference is with 2. So, there are 2 smart
> pointers
> > using the thread. One is mine, and the other?
> >
> > I found this really strange code in PosixThread.cpp
> >
> > PosixThread::start()
> >
> > ...
> >     // Create reference
> >     shared_ptr<PthreadThread>* selfRef = new shared_ptr<PthreadThread>();
> >     *selfRef = self_.lock();
> > ...
> >
> > So, it just creates a local scope smart pointer the let it leak, so, the
> > reference count will not be decreased letting my thread leaking.
> >
> > Can some one explain me that?
> >
> > Sorry by my poor english.
> >
> > Thanks.
> >
>



-- 
Vinicius Tinti -> viniciustinti@gmail.com, tinti@comp.eng.br,
vtintipo@gmail.com

"Here Goes Nothing"
"Beware of bugs in the above code; I have only proved it correct, not tried
it."
"Only those who attempt the absurd can achieve the impossible."
"Never underestimate the power of very stupid people in large groups."
"Simplicity is the ultimate sophistication"
"Beware, I know Ruby-FU"

Re: Confusing code in PosixThread.start() [c++]

Posted by David Reiss <dr...@facebook.com>.
The author of that code had this to say:

A Thread has a weak reference to itself and a reference (shared_ptr)  
to it's Runnable object

A Runnable has a weak reference to the Thread object.

If a thread creator gives up it's reference to Runnable immediately  
after creating the thread, this guarantees that the Runnable WILL NOT  
be GC'ed.
If a thread creator gives up both its Runnable and Thread before  
invoking Thread.start, this guarantees that both Thread and Runnable  
WILL be GC'ed

In PthreadThread::start the weak ref is converted into a heap-based  
regular ref so that the thread doesn't get GC'ed before  
PthreadThread:threadMain is entered.  In threadMain, the heap-based  
regular ref is replaced by a stack-based ref.
Doing this means that the Thread and Runnable objects WILL NOT get  
GCed until threadMain exits, even if all other references have been  
given up.

So the answer to Vinicius' question is the 2 references are for 1) the  
creator thread and 2) the thread created by  PosixThread::start.  This  
is not a leak and works as designed.  I'm not clear why this would  
show up as a leak in valgrind unless he is not doing a clean exit.


Vinicius Tinti wrote:
> Hello,
> 
> I detect some memory leak in my application using valgrind.
> 
> The leak happen with PosixThread, after call PosixThread.start() the
> reference count of shared_ptr increases by one. When the thread
> loses it scope the count reference is with 2. So, there are 2 smart pointers
> using the thread. One is mine, and the other?
> 
> I found this really strange code in PosixThread.cpp
> 
> PosixThread::start()
> 
> ...
>     // Create reference
>     shared_ptr<PthreadThread>* selfRef = new shared_ptr<PthreadThread>();
>     *selfRef = self_.lock();
> ...
> 
> So, it just creates a local scope smart pointer the let it leak, so, the
> reference count will not be decreased letting my thread leaking.
> 
> Can some one explain me that?
> 
> Sorry by my poor english.
> 
> Thanks.
>