Re: [Flightgear-devel] Threadsafe properties (deadlock)

[Julian S. <ju...@op...>]

On Sat, 3 Jun 2023 17:03:26 +0200
Nikolai Verner Christensen <emp...@gm...> wrote:

> Someone found out this:
> 
> "When I set "SG_PROPERTY_LOCKING_VERBOS" to 1, f16 firing missiles
> became normal, FightGear nightly would not stuck.
> Maybe answer is in simgear/propscxx Line 328
> <https://sourceforge.net/p/flightgear/simgear/ci/next/tree/simgear/props/props.cxx>
> "
> 
> >  

Setting SG_PROPERTY_LOCKING_VERBOSE=1 makes a very small change to the
behaviour of a single function, SGPropertyLock::acquire_internal(),
making it attempt a non-blocking lock acquisition first:

void acquire_internal(const SGPropertyNode& node, bool shared)
{
    if (s_property_locking_verbose)
    {
        bool ok;
        if (shared) ok = node._mutex.try_lock_shared();
        else ok = node._mutex.try_lock();
        if (ok) return;
        std::cerr << (shared ? "    shared" : " exclusive") << " lock contention" << "\n";
    }

    if (shared) node._mutex.lock_shared();
    else node._mutex.lock();
}

[The actual code is a little different, in an attempt to optimise the
s_property_locking_verbose==false case, and also to catch exceptions in
the s_property_locking_verbose==true case.]

I'm fairly sure the code is correct in either case, but would be
delighted if someone could spot a bug.

It's possible that a failed non-blocking acquisition might cause a
thread switch, which could perhaps avoid a deadlock in a buggy
programme. In which case we'll need to see the diagnostics about what
properties are contended.

Another possibility is that acquisition is throwing an exception, which
we will re-throw, but it's being swallowed by calling code. But i'd
expect reports of a std::cerr diagnostic in this case.

Finally i guess it's possible that there is a bug in the C++ mutex
implementation, but this seems very unlikely.


Overall, setting SG_PROPERTY_LOCKING_VERBOSE=1 is not a solution to the
problem i think.

To make progress here, i think we're going to need to see the
diagnostics that i asked for previously.


- Jules

-- 
http://op59.net