Re: Re: Re: A possible race condition in Packet::invalidateHeader()

Hi Atul,

I read the article you referred to. It was quite interesting! If I understood correctly, it is recommended that all thread shared data be volatile. volatile is not needed if access to the data is synchronized using a semaphore for example.

In our case the data is shared, but not the pointer, so maybe there is no need to declare the pointer volatile. I’m not sure about that! Trolltech do not declare the pointer volatile in Qt but that does not mean it wouldn’t make sense to do so.

After you brought up the topic about thread safety of Packet, I looked again into Packet and remembered what my concerns were before. In a copy constructor or assignment operator is done this:


Data *x = other.d;
x->ref.ref();

Now what would happen if the object from which we copy gets deleted from another thread right in between? Maybe the scenario I’m giving is a bad and trying to copy an object that is about to be deleted is inherently bad and reference counting simply cannot help against that! Any ideas?

Regards,

Peter

atul wrote:Dear Peter

My response in bold

Peter Dimov wrote:

Hi Atul,

Did you verify that a race occurs?

Not with some tool like DRDT on solaris 10 ( Btw, it is a great tool and could be the sole reason to port any multi threaded program to solaris ;-)

See http://developers.sun.com/sunstudio/downloads/drdt/drdt_index.html

I am planning to do it once we start giving out MT releases ;-)

I that should not be the case.
…. Yes ;-)
Let me know if I’m wrong!

I think I am wrong… However I am not too sure if we should not stick a volatile keyword before

volatile Data *d;

in Packet…

See http://www.ddj.com/dept/cpp/184403766 for example

Let me know if I am wrong ( Anything multi-threaded should be handled like a burning hot iron rod, IMHO ;-)

It’ll be great if you can take a look at Packet from the multi-threading perspective.

Sure ;-)

Regards,

Peter

atul wrote:Dear all

While doing the refactoring of this function, the race poped out …. ;-)

if (!x->ref.deref())

delete x;

Now what happens thread T1 has finish checking the above conditional and is then pre empted … T2 runs the the same conditional and deletes x. Thread T1 resumes and deletes x

As we are using thread safe functions to swap ( if I am right ) the pointers, we should consider all races

Adding multi-threading at some later point in the code is very hard …

—- cheers atul

Would you like to post a relpy?


This post is a reply to:
Re: Re: A possible race condition in Packet::invalidateHeader()
Dear Peter My response in bold Peter Dimov wrote: Hi Atul, Did you verify that a race occurs? Not with some tool like DRDT on solaris 10 ( Btw, it is a great tool and (more...)

No follow-ups yet.