Handle class usage in multi-thread environment
Hello,
After readed class Handle's implemention in Handle.h, I have a question about the usage in multithread environment. It seems the copy constructor is not thread safe.
e.g.
template<typename Y>
Handle(const Handle<Y>& r)
{
this->_ptr = r._ptr;
if(this->_ptr)
{
incRef(this->_ptr);
}
}
Thread 1:
ptr1 = ptr2;
Thread 2:
ptr2 = 0; //when ptr2 has only 1 reference count.
After thread 1 finished executing this->_ptr = r._ptr;
it happens a task switch to thread 2.
then back to thread 1, now this->_ptr point to a invalid address.
I think add a lock will be better:
private:
LOCK lock_;
public:
T* __lock_addref()
{
lock_guard(this->lock_);
if (this->_ptr)
this->_ptr->__addref();
return this->_ptr;
}
template<typename Y>
Handle(const Handle<Y>& r)
{
this->_ptr = (const_cast<usys_smartptr<Y>&>(r)).__lock_addref();
}
Is it neccessary?
Thanks,
Yongming
After readed class Handle's implemention in Handle.h, I have a question about the usage in multithread environment. It seems the copy constructor is not thread safe.
e.g.
template<typename Y>
Handle(const Handle<Y>& r)
{
this->_ptr = r._ptr;
if(this->_ptr)
{
incRef(this->_ptr);
}
}
Thread 1:
ptr1 = ptr2;
Thread 2:
ptr2 = 0; //when ptr2 has only 1 reference count.
After thread 1 finished executing this->_ptr = r._ptr;
it happens a task switch to thread 2.
then back to thread 1, now this->_ptr point to a invalid address.
I think add a lock will be better:
private:
LOCK lock_;
public:
T* __lock_addref()
{
lock_guard(this->lock_);
if (this->_ptr)
this->_ptr->__addref();
return this->_ptr;
}
template<typename Y>
Handle(const Handle<Y>& r)
{
this->_ptr = (const_cast<usys_smartptr<Y>&>(r)).__lock_addref();
}
Is it neccessary?
Thanks,
Yongming
0
Comments
Read and write access to the same IceUtil::Handle instance from multiple threads is not safe, it might result in undefined behavior. You need to add some synchronization if you want to read and modify the same handle instance from different threads.
Adding this synchronization to the IceUtil::Handle implementation would have a performance and memory cost. Since most applications don't need such thread safety it's better to not add it.
Cheers,
Benoit.