Fix self-deadlock when acquiring a shared SyncObject latch

[MochalovAlexey]

This fixes a possible engine hang when the current thread tries to acquire
a SYNC_SHARED latch on a SyncObject that it already owns exclusively.

The hang was observed several times in a customer environment, but so far
it has not been possible to reproduce it manually with an unmodified build.
The problematic lock sequence was reproduced only by temporarily modifying
the code and running it in a debugger.

Instead of waiting on itself, SyncObject now detects this condition and
raises a dedicated error. Page-cache latches are unwound when the exception
propagates through VIO record operations, backout and savepoint cleanup.

The fix was originally implemented for the 3v and has been adapted
to the Firebird 5.

[hvlad]

The hang was observed several times in a customer environment

No memory dump or stack trace ? What Firebird version is affected ?

Instead of waiting on itself, SyncObject now detects this condition and
raises a dedicated error. Page-cache latches are unwound when the exception
propagates through VIO record operations, backout and savepoint cleanup.

What about other usages of SyncObject ? Other usages of bdb_sync ?

Excuse me, but it looks like desperate attempt to workaround bug instead of properly fix it.
Also, I see no guarantee that it covers all possible cases - as there is no knowledge when and how it happens.

[MochalovAlexey]

The original hang was observed in a customer environment running our fork of Firebird 3. However, the underlying problem is also present in upstream Firebird 3 and can be reproduced artificially with the debugger. I have attached both the reproducer deadlock.patch and the resulting syncobject_deadlock.txt.

To reproduce it, set a breakpoint at:
if (forceErrorDebugFlag ||
and change forceErrorDebugFlag to true. Then add enough records to any table to expand the database file.

I did not create a PR for upstream Firebird 3 because that version is no longer supported.I was unable to reproduce this issue in Firebird 5 or later versions, but after analyzing the code, I concluded that a similar situation is possible.

The concrete incident (on FB3 fork) was a self-deadlock during rollback/savepoint cleanup after an I/O error. This was not a lock-manager deadlock: the thread was waiting on a page-cache SyncObject.
The sequence was as follows:

DPM_store(...)
  -> locate_space(...)
     -> extend_relation(...)
        -> get_pointer_page(..., LCK_write, pag_pointer)
           acquires SYNC_EXCLUSIVE on the pointer page

        -> DPM_allocate(...)
           -> PAG_allocate(...)
              -> PAG_allocate_pages(...)
                 -> CCH_FETCH(..., LCK_write, pag_pages)
                    acquires the PIP page

                 -> ensureDiskSpace(...)
                    -> PageSpace::extend(...)
                       -> PageSpace::maxAlloc()
                          or PageSpace::actAlloc()
                          -> PIO_get_number_of_pages(...)
                             -> nt_error("GetFileSize", ...)
                                or unix_error("ioctl(BLKGETSIZE)", ...)
                                throws Firebird::status_exception

The exception is then caught by EXE_looper():

EXE_looper(...)
  catch (const Firebird::Exception&)
    -> ++tra_save_point->sav_verb_count
    -> VIO_verb_cleanup(...)

During cleanup, the same thread attempts to read the same pointer page:

VIO_verb_cleanup(...)
  -> DPM_get(...)
     -> get_pointer_page(..., LCK_read)
        -> CCH_FETCH(..., LCK_read, pag_pointer)
           -> BufferDesc::addRef(..., SYNC_SHARED)
              -> bdb_syncPage.lock(..., SYNC_SHARED)
                 hangs

The root cause is that extend_relation() has already acquired the pointer page for writing. On the normal execution path it is released later by CCH_HANDOFF() or CCH_RELEASE(). An exception from PIO_get_number_of_pages() interrupts execution before those windows are released.
This exception can originate from:
nt_error("GetFileSize", ..., 0) on Windows;
unix_error("ioctl(BLKGETSIZE)", ..., NULL) on POSIX.
Because the status-vector argument is null, these functions call ERR_post(), which throws Firebird::status_exception.
The thread then deadlocks on itself because SyncObject only handles recursive SYNC_EXCLUSIVE acquisition by the exclusive owner:

if (thread == exclusiveThread)
{
    ++monitorCount;
    reason(from);
    return true;
}

There is no equivalent handling for a SYNC_SHARED request made by the current exclusive owner. Consequently, the thread is added to the wait queue of a SyncObject for which it is itself the exclusive owner.
The dump showed the following:
The affected pointer page was 1:2203756, with pag_type = 4 (pag_pointer).
bdb_syncPage.lockState.counter was -1, meaning that the page was held exclusively.
bdb_syncPage.exclusiveThread.threadId was 7356.
bdb_syncPage.waitingThreads.threadId was also 7356.
The requested latch type was SYNC_SHARED.
this == this->bdb_exclusive->tdbb_bdbs.data[0], confirming that the same thread_db still held the BDB.

Regarding other uses of SyncObject: the deadlock detection itself is generic, but the latch-unwinding part of the proposed change is specific to page-cache BDBs tracked in tdbb_bdbs.
I am not claiming that this solves arbitrary exception-safety problems in all SyncObject users, nor that every possible bdb_sync path is covered.

The dedicated SyncObject error is primarily intended to prevent an undiagnosable infinite wait and expose an error.

[hvlad]

The original hang was observed in a customer environment running our fork of Firebird 3. However, the underlying problem is also present in upstream Firebird 3 and can be reproduced artificially with the debugger. I have attached both the reproducer deadlock.patch and the resulting syncobject_deadlock.txt.

It is not clear why PIO_get_number_of_pages() failed.
Potential issue when fstat() failed with EINTR was fixed in v4, btw.

To reproduce it, set a breakpoint at: if (forceErrorDebugFlag || and change forceErrorDebugFlag to true. Then add enough records to any table to expand the database file.

I did not create a PR for upstream Firebird 3 because that version is no longer supported.I was unable to reproduce this issue in Firebird 5 or later versions, but after analyzing the code, I concluded that a similar situation is possible.

The concrete incident (on FB3 fork) was a self-deadlock during rollback/savepoint cleanup after an I/O error. This was not a lock-manager deadlock: the thread was waiting on a page-cache SyncObject. The sequence was as follows:

DPM_store(...)
  -> locate_space(...)
     -> extend_relation(...)
        -> get_pointer_page(..., LCK_write, pag_pointer)
           acquires SYNC_EXCLUSIVE on the pointer page

        -> DPM_allocate(...)
           -> PAG_allocate(...)
              -> PAG_allocate_pages(...)
                 -> CCH_FETCH(..., LCK_write, pag_pages)
                    acquires the PIP page

                 -> ensureDiskSpace(...)
                    -> PageSpace::extend(...)
                       -> PageSpace::maxAlloc()
                          or PageSpace::actAlloc()
                          -> PIO_get_number_of_pages(...)
                             -> nt_error("GetFileSize", ...)
                                or unix_error("ioctl(BLKGETSIZE)", ...)
                                throws Firebird::status_exception

The exception is then caught by EXE_looper():

EXE_looper(...)
  catch (const Firebird::Exception&)
    -> ++tra_save_point->sav_verb_count
    -> VIO_verb_cleanup(...)

It is much better is to handle this case at PAG_allocate_pages() or ensureDiskSpace or even deeper in call stack.
Perhaps it will make sense to pass status into PIO_get_number_of_pages() and not raise exception when not needed (as in this case)