Quoting Matthieu Fertr� <[EMAIL PROTECTED]>:
> I'm back to point the different potential race conditions that can
> appear with the proposed implementation (current implementation is also
> sensible to race conditions).

Indeed, I didn't try to address the threading problems.

> Here are some typical race conditions:
> 1) thread1 and thread2 ask for Object::GetInstance(), at the time of the
> requests, "singleton" is NULL. The 2 threads will start to create an
> instance, and one will be lost...

Or merge info into the same static list (happened with team and network: 2
instances of the same team in the list).

> 2) thread1 ask for Object::GetInstance() and thread2 ask for
> OtherObject::GetInstance(). If the two objects are null,
> singletons.push_back(this) may be called simultaneously and we have lost
> an object (C++ STL is not thread-safe).

Yes, adding a lock around push_back and remove is safer. The cost is not that
big, because it is during creation. However, it increases the likeliness of the
first problem you mention above. Probably still worth it, as

> 3) thread1 tries to access to the singleton instance of Object and
> "singleton" is currently not NULL. BUT, thread 2 maybe deleting
> singleton itself, the problem is that singleton is set to NULL in the
> Singleton destructor. It means that thread1 can access to an object that
> is in destruction...

The only solution would be to also add a lock on instance creation and
destruction and also in GetInstance. Fortunately, if we decide to change the
code, it is mostly changing GetInstance method in the base templated Singleton
class.

Maybe worth investigating, but the current code is not that much more buggy than
currently on the thread matters.

> Sure, such race conditions will not arrive every days but we have
> already faced to the first one, so we have to take care.

Definitely. And there may be more on network initialization that were not
reported or not seen yet.

I'll first apply the patch as is this weekend (unless another developer names a
strong distaste), then afterwards add incrementally whatever we think must be
added.

_______________________________________________
Wormux-dev mailing list
Wormux-dev@gna.org
https://mail.gna.org/listinfo/wormux-dev

Répondre à