runtime: prevent refcount underflow on extra shutdown calls - #7299

I'm not sure I understand why this is useful? I mean, I guess I understand at a meta level, but what's the concrete use case here? Because it feels like a thing that you should be able to track.

But if not, it also feels like we have a test and set race. Should we just check for <= 0 on the decrement instead?

Thanks for the quick reply. I found this while checking whether cleanup/shutdown functions can be called idly, much like the free(NULL).

The concrete issue is the API contract. As the documentation says, git_libgit2_shutdown() returns the number of remaining init count or an error code. Underflow produces a negative value that is neither, violating the documented semantics.

it also feels like we have a test and set race.

I don't think there's a race, both the get and dec are inside the lock.

Should we just check for <= 0 on the decrement instead?

In this case, the counter has already gone negative. That would also require git_libgit2_init() to be changed.

I just created #7303 to illustrate that by allowing this underflow to happen, we can end up in a situation where code that looks perfectly innocuous (i.e. if (git_libgit2_init() < 0) return;) can cause us to use a de-initialized library, should git_libgit2_shutdown() be called exactly one too many times beforehand.

In this case, the counter has already gone negative. That would also require git_libgit2_init() to be changed.

Well, if this a pattern that we want to support, then we should probably change it. This introduces a TOCTOU bug which is probably worse overall than the status quo.

But libgit2 isn't designed for reinitialization after being shutdown. Shutdown is the terminal case, and honestly, it's fine to just... not call it. The OS will obviously tidy up the few handles that libgit2 leaves open.

Not doing so could interfere with valgrind, or similar things in your test harness, but this seems to be exactly the sort of synthetic behavior that I would expect to be easy to instrument and not reentrant.

But hey, I don't know what it is that you're trying to do. So, what are you trying to do here?