Eliminate defects found by Coverity

[martyone]

The last commit waits for an issue with user-defined models to be fixed on Coverity Scan side. Otherwise it is ready for review.

[monich]

I'm puzzled by "The code does not seem to be executed often enough" commit. That particular code is executed as often as local objects are being registered by the program using this library. There's no way of knowing how often that happens, how can one claim that it's not often enough. That's nonsense.

And why is it "generally unsafe"? I'm not seeing anything obviously unsafe about that check and I'm curious (I really am) what exactly is meant by that.

The purpose of the double-check is to avoid unnecessarily grabbing the looper mutex.

[martyone]

I'm puzzled by "The code does not seem to be executed often enough" commit. That particular code is executed as often as local objects are being registered by the program using this library. There's no way of knowing how often that happens, how can one claim that it's not often enough. That's nonsense.

That's why I wrote "does not seem to be" instead of "is not". It was the best possible assessment I could do together with @krnlyng with moderate effort, before going to you to hear your most experienced opinion :)

An uneducated but natural expectation is that the number of times objects are used exceeds significantly the number of times objects are created, so the overhead of locking during object creation is naturally expected to be rather low.

And why is it "generally unsafe"? I'm not seeing anything obviously unsafe about that check and I'm curious (I really am) what exactly is meant by that.

I stated it is "generally" unsafe, because I couldn't find any resources dedicated specifically to the C language case, but there are many resources describing the difficulties with getting double-checked locking reliably and portably in C++, Java, etc. https://www.google.com/search?q=double-checked+locking+broken

As far as I understand the described issues, I think it is rather unlikely it could happen in (pure) C at least in our specific case, but this is an area I really do not feel confident in. Who knows what changes may the code undergo during the optimization phase. Can you prove it is safe? At the same time it holds that preliminary optimization is bad, so we should prove that this optimization pays off if we want Coverity to stop mentioning it and then claim the customers Coverity found no issues with our code.

[martyone]

Dropped the commit replaced by PR #127.

[monich]

I still don't see any problem with the code. The scenarios where double-checking could be a problem involve broken compilers (which optimize with disastrous side effects) which results in synchronization being generally broken anyway, which of course breaks perfectly synchronized lazy initialization as well.

Of course I can't prove that compilers are not broken. But I can prove that there's no problem with the code.

And it's generally unsafe to use broken compilers, no arguing about that)

I do agree though, that the effect of this optimization is negligible. But pretty much the same thing can be said about almost any optimization. Very rarely programs are slow because of one local bottleneck. That happens but it's easy to fix. Most programs are slow because many (thousands, tens of thousands, millions) things are done sub-optimally. And that's much harder, if not impossible, to fix because you'd have to fix all those places, and the effect of each change would be negligible, to the point that it becomes easier and more efficient to just write a new program from scratch. Alternatively, you can try to write optimal code. I prefer the latter because I don't like rewriting the same thing over and over again.

I hope that explains why I'm convinced that (reasonably) small optimizations is generally a good thing to do.

It's ok to remove the check since there's a clear reason for that, albeit non-technical. But please update the comment stating that it's being removed for non-technical reasons - just to silence Coverity, not because there was something wrong with the code.

[martyone]

Updated commit message as suggested by @monich .

[monich]

Updated commit message as suggested by @monich .

thanks))

[monich]

Won't tag it yet, there's another PR pending...

[martyone]

@monich it's been some time - could you please tag it now?

[monich]

@monich it's been some time - could you please tag it now?

Done!