One NodeShared per process

[joxoby]

https://github.com/orgs/ignitionrobotics/projects/3#card-38255453

[joxoby]

We may want to make this Uuid comparison faster

[caguero]

Thanks for the contribution. I've been doing some testing and I have two major concerns:

1. As far as I understand, the new implementation of NodeShared()::Instance() doesn't behave as a singleton anymore. Two calls to Instance() within the same process are going to instantiate two NodeShared objects which defeats the purpose of the singleton.

if (instance == nullptr || instance->pUuid != Uuid().ToString())

The expression instance->pUuid != Uuid().ToString() is going to evaluate to true for every new Instance() call. This breaks the current functionality of the library. The basic publisher/subscriber example doesn't work anymore.

2. Assuming that the patch would behave as a singleton, I don't think is thread safe anymore. However, doing the static initialization of instance directly is guarantee to be thread-safe.

[joxoby]

Hi Carlos,

1. You're right, it was a confusion of mine regarding Uuid. What I meant to codify was something like

if (instance == nullptr || instance->pid != getpid())

2. We can easily add a lock_guard. WDYT?

[caguero]

instance == nullptr || instance->pid != getpid()

Do you mind to update this pull request with a working prototype? It'll help to wrap my head around it. Thanks!

[joxoby]

With the last change, the ignition-transport test are passing but it does cause problems with ign-gazebo. Will update.

[caguero]

I'll decline this pull request for now until there's a better patch that keeps all tests and examples working. Feel free to reopen if that's the case.