From e94879ed116b358dc71d9711c33bd619c387b9d2 Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Mon, 3 Jun 2024 16:05:35 +0300 Subject: [PATCH] Create RWLock for file descriptor, so we can be sure that some other thread won't close it while we're using it. For Classic Server you can change forced write mode only with no connections. --- src/include/firebird/impl/msg/jrd.h | 1 + src/include/gen/Firebird.pas | 1 + src/jrd/Database.cpp | 6 ++- src/jrd/nbak.cpp | 8 +++- src/jrd/os/pio.h | 6 ++- src/jrd/os/posix/unix.cpp | 27 ++++++++--- src/jrd/os/win32/winnt.cpp | 73 +++++++++-------------------- src/jrd/pag.cpp | 43 ++++++++++++----- 8 files changed, 92 insertions(+), 73 deletions(-) diff --git a/src/include/firebird/impl/msg/jrd.h b/src/include/firebird/impl/msg/jrd.h index 535d4e4f438..769d5e439ba 100644 --- a/src/include/firebird/impl/msg/jrd.h +++ b/src/include/firebird/impl/msg/jrd.h @@ -968,3 +968,4 @@ FB_IMPL_MSG(JRD, 965, ods_upgrade_err, -901, "HY", "000", "ODS upgrade failed wh FB_IMPL_MSG(JRD, 966, bad_par_workers, -924, "HY", "000", "Wrong parallel workers value @1, valid range are from 1 to @2") FB_IMPL_MSG(JRD, 967, idx_expr_not_found, -902, "42", "000", "Definition of index expression is not found for index @1") FB_IMPL_MSG(JRD, 968, idx_cond_not_found, -902, "42", "000", "Definition of index condition is not found for index @1") +FB_IMPL_MSG(JRD, 969, forced_write_change_err, -902, "42", "000", "Cannot change forced write mode for classic server while it has connections") diff --git a/src/include/gen/Firebird.pas b/src/include/gen/Firebird.pas index a11336e842a..d702436ea63 100644 --- a/src/include/gen/Firebird.pas +++ b/src/include/gen/Firebird.pas @@ -5700,6 +5700,7 @@ IProfilerStatsImpl = class(IProfilerStats) isc_bad_par_workers = 335545286; isc_idx_expr_not_found = 335545287; isc_idx_cond_not_found = 335545288; + isc_forced_write_change_err = 335545289; isc_gfix_db_name = 335740929; isc_gfix_invalid_sw = 335740930; isc_gfix_incmp_sw = 335740932; diff --git a/src/jrd/Database.cpp b/src/jrd/Database.cpp index 57b84ffc968..3df6852e577 100644 --- a/src/jrd/Database.cpp +++ b/src/jrd/Database.cpp @@ -130,7 +130,11 @@ namespace Jrd const PageSpace* const pageSpace = dbb_page_manager.findPageSpace(DB_PAGE_SPACE); UCharBuffer buffer; - os_utils::getUniqueFileId(pageSpace->file->fil_desc, buffer); + { + jrd_file* file = pageSpace->file; + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + os_utils::getUniqueFileId(file->fil_desc, buffer); + } auto ptr = dbb_file_id.getBuffer(2 * buffer.getCount()); for (const auto val : buffer) diff --git a/src/jrd/nbak.cpp b/src/jrd/nbak.cpp index 3e5f86cd966..edd9fead067 100644 --- a/src/jrd/nbak.cpp +++ b/src/jrd/nbak.cpp @@ -309,9 +309,13 @@ void BackupManager::beginBackup(thread_db* tdbb) PageSpace* pageSpace = database->dbb_page_manager.findPageSpace(DB_PAGE_SPACE); const char* func = NULL; - if (os_utils::fstat(pageSpace->file->fil_desc, &st) != 0) { - func = "fstat"; + jrd_file* file = pageSpace->file; + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (os_utils::fstat(file->fil_desc, &st) != 0) + { + func = "fstat"; + } } while (!func && fchown(diff_file->fil_desc, st.st_uid, st.st_gid) != 0) diff --git a/src/jrd/os/pio.h b/src/jrd/os/pio.h index f5ff6f4a178..064f91358bc 100644 --- a/src/jrd/os/pio.h +++ b/src/jrd/os/pio.h @@ -46,8 +46,9 @@ class jrd_file : public pool_alloc_rpt ULONG fil_max_page; // Maximum page number in file USHORT fil_sequence; // Sequence number of file USHORT fil_fudge; // Fudge factor for page relocation - int fil_desc; + int fil_desc; // Lock fil_desc_lock before using Firebird::Mutex fil_mutex; + mutable Firebird::RWLock fil_desc_lock; USHORT fil_flags; SCHAR fil_string[1]; // Expanded file name }; @@ -74,8 +75,9 @@ class jrd_file : public pool_alloc_rpt ULONG fil_max_page; // Maximum page number in file USHORT fil_sequence; // Sequence number of file USHORT fil_fudge; // Fudge factor for page relocation - HANDLE fil_desc; // File descriptor + HANDLE fil_desc; // File descriptor, lock fil_desc_lock before using Firebird::RWLock* fil_ext_lock; // file extend lock + mutable Firebird::RWLock fil_desc_lock; USHORT fil_flags; SCHAR fil_string[1]; // Expanded file name }; diff --git a/src/jrd/os/posix/unix.cpp b/src/jrd/os/posix/unix.cpp index 7a146367750..481b8a41ae7 100644 --- a/src/jrd/os/posix/unix.cpp +++ b/src/jrd/os/posix/unix.cpp @@ -192,6 +192,8 @@ void PIO_close(jrd_file* main_file) for (jrd_file* file = main_file; file; file = file->fil_next) { + WriteLockGuard writeGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc && file->fil_desc != -1) { close(file->fil_desc); @@ -336,6 +338,8 @@ void PIO_extend(thread_db* tdbb, jrd_file* main_file, const ULONG extPages, cons int r; for (r = 0; r < IO_RETRY; r++) { + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + int err = fallocate(file->fil_desc, 0, filePages * pageSize, extendBy * pageSize); if (err == 0) break; @@ -393,6 +397,8 @@ void PIO_flush(thread_db* tdbb, jrd_file* main_file) for (jrd_file* file = main_file; file; file = file->fil_next) { + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc != -1) { // This really should be an error @@ -432,6 +438,7 @@ void PIO_force_write(jrd_file* file, const bool forcedWrites, const bool notUseF const int control = (forcedWrites ? SYNC : 0) | (notUseFSCache ? O_DIRECT : 0); + WriteLockGuard writeGuard(file->fil_desc_lock, FB_FUNCTION); #ifndef FCNTL_BROKEN if (fcntl(file->fil_desc, F_SETFL, control) == -1) { @@ -479,6 +486,8 @@ ULONG PIO_get_number_of_pages(const jrd_file* file, const USHORT pagesize) * **************************************/ + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc == -1) unix_error("fstat", file, isc_io_access_err); @@ -545,6 +554,8 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length) PageSpace* pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE); jrd_file* file = pageSpace->file; + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc == -1) unix_error("PIO_header", file, isc_io_read_err); @@ -637,6 +648,8 @@ USHORT PIO_init_data(thread_db* tdbb, jrd_file* main_file, FbStatusVector* statu { if (!(file = seek_file(file, &bdb, &offset, status_vector))) return false; + + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); if ((written = os_utils::pwrite(file->fil_desc, zero_buff, to_write, LSEEK_OFFSET_CAST offset)) == to_write) break; if (written < 0 && !SYSCALL_INTERRUPTED(errno)) @@ -751,13 +764,13 @@ bool PIO_read(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, SINT64 bytes; FB_UINT64 offset; + EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc == -1) return unix_error("read", file, isc_io_read_err, status_vector); Database* const dbb = tdbb->getDatabase(); - - EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - const SLONG size = dbb->dbb_page_size; for (i = 0; i < IO_RETRY; i++) @@ -803,13 +816,13 @@ bool PIO_write(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, SINT64 bytes; FB_UINT64 offset; + EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc == -1) return unix_error("write", file, isc_io_write_err, status_vector); Database* const dbb = tdbb->getDatabase(); - - EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - const SLONG size = dbb->dbb_page_size; for (i = 0; i < IO_RETRY; i++) @@ -858,6 +871,8 @@ static jrd_file* seek_file(jrd_file* file, BufferDesc* bdb, FB_UINT64* offset, break; } + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + if (file->fil_desc == -1) { unix_error("lseek", file, isc_io_access_err, status_vector); diff --git a/src/jrd/os/win32/winnt.cpp b/src/jrd/os/win32/winnt.cpp index ad88cc3b02a..67b1bd75029 100644 --- a/src/jrd/os/win32/winnt.cpp +++ b/src/jrd/os/win32/winnt.cpp @@ -54,49 +54,6 @@ #include -namespace Jrd { - -class FileExtendLockGuard -{ -public: - FileExtendLockGuard(Firebird::RWLock* lock, bool exclusive) : - m_lock(lock), m_exclusive(exclusive) - { - if (m_exclusive) { - fb_assert(m_lock); - } - if (m_lock) - { - if (m_exclusive) - m_lock->beginWrite(FB_FUNCTION); - else - m_lock->beginRead(FB_FUNCTION); - } - } - - ~FileExtendLockGuard() - { - if (m_lock) - { - if (m_exclusive) - m_lock->endWrite(); - else - m_lock->endRead(); - } - } - -private: - // copying is prohibited - FileExtendLockGuard(const FileExtendLockGuard&); - FileExtendLockGuard& operator=(const FileExtendLockGuard&); - - Firebird::RWLock* const m_lock; - const bool m_exclusive; -}; - - -} // namespace Jrd - using namespace Jrd; using namespace Firebird; @@ -174,6 +131,7 @@ void PIO_close(jrd_file* main_file) **************************************/ for (jrd_file* file = main_file; file; file = file->fil_next) { + WriteLockGuard writeGuard(file->fil_desc_lock, FB_FUNCTION); maybeCloseFile(file->fil_desc); } } @@ -271,7 +229,7 @@ void PIO_extend(thread_db* tdbb, jrd_file* main_file, const ULONG extPages, cons return; EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - FileExtendLockGuard extLock(main_file->fil_ext_lock, true); + WriteLockGuard extLock(main_file->fil_ext_lock, FB_FUNCTION); ULONG leftPages = extPages; for (jrd_file* file = main_file; file && leftPages; file = file->fil_next) @@ -283,6 +241,7 @@ void PIO_extend(thread_db* tdbb, jrd_file* main_file, const ULONG extPages, cons { const ULONG extendBy = MIN(fileMaxPages - filePages + file->fil_fudge, leftPages); + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); HANDLE hFile = file->fil_desc; LARGE_INTEGER newSize; @@ -317,7 +276,10 @@ void PIO_flush(thread_db* tdbb, jrd_file* main_file) EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); for (jrd_file* file = main_file; file; file = file->fil_next) + { + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); FlushFileBuffers(file->fil_desc); + } } @@ -344,7 +306,9 @@ void PIO_force_write(jrd_file* file, const bool forceWrite, const bool notUseFSC const int writeMode = (file->fil_flags & FIL_readonly) ? 0 : GENERIC_WRITE; const bool sharedMode = (file->fil_flags & FIL_sh_write); - HANDLE& hFile = file->fil_desc; + WriteLockGuard writeGuard(file->fil_desc_lock, FB_FUNCTION); + + HANDLE& hFile = file->fil_desc; maybeCloseFile(hFile); hFile = CreateFile(file->fil_string, GENERIC_READ | writeMode, @@ -399,6 +363,8 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length) PageSpace* pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE); jrd_file* file = pageSpace->file; + + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); HANDLE desc = file->fil_desc; OVERLAPPED overlapped; @@ -441,7 +407,7 @@ USHORT PIO_init_data(thread_db* tdbb, jrd_file* main_file, FbStatusVector* statu Database* const dbb = tdbb->getDatabase(); EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - FileExtendLockGuard extLock(main_file->fil_ext_lock, false); + ReadLockGuard extLock(main_file->fil_ext_lock, FB_FUNCTION); // Fake buffer, used in seek_file. Page space ID doesn't matter there // as we already know file to work with @@ -474,6 +440,8 @@ USHORT PIO_init_data(thread_db* tdbb, jrd_file* main_file, FbStatusVector* statu const DWORD to_write = (DWORD) write_pages * dbb->dbb_page_size; DWORD written; + + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); BOOL ret = WriteFile(file->fil_desc, zero_buff, to_write, &written, &overlapped); if (!ret) { @@ -579,12 +547,13 @@ bool PIO_read(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, const DWORD size = dbb->dbb_page_size; EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - FileExtendLockGuard extLock(file->fil_ext_lock, false); + ReadLockGuard extLock(file->fil_ext_lock, FB_FUNCTION); OVERLAPPED overlapped; if (!(file = seek_file(file, bdb, &overlapped))) return false; + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); HANDLE desc = file->fil_desc; DWORD actual_length; @@ -756,13 +725,14 @@ bool PIO_write(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, const DWORD size = dbb->dbb_page_size; EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - FileExtendLockGuard extLock(file->fil_ext_lock, false); + ReadLockGuard extLock(file->fil_ext_lock, FB_FUNCTION); OVERLAPPED overlapped; file = seek_file(file, bdb, &overlapped); if (!file) return false; + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); HANDLE desc = file->fil_desc; DWORD actual_length; @@ -792,6 +762,9 @@ ULONG PIO_get_number_of_pages(const jrd_file* file, const USHORT pagesize) * Compute number of pages in file, based only on file size. * **************************************/ + + ReadLockGuard readGuard(file->fil_desc_lock, FB_FUNCTION); + HANDLE hFile = file->fil_desc; DWORD dwFileSizeHigh; @@ -800,7 +773,7 @@ ULONG PIO_get_number_of_pages(const jrd_file* file, const USHORT pagesize) if ((dwFileSizeLow == INVALID_FILE_SIZE) && (GetLastError() != NO_ERROR)) nt_error("GetFileSize", file, isc_io_access_err, 0); - const ULONGLONG ullFileSize = (((ULONGLONG) dwFileSizeHigh) << 32) + dwFileSizeLow; + const ULONGLONG ullFileSize = (((ULONGLONG) dwFileSizeHigh) << 32) + dwFileSizeLow; return (ULONG) ((ullFileSize + pagesize - 1) / pagesize); } @@ -835,7 +808,7 @@ static jrd_file* seek_file(jrd_file* file, page -= file->fil_min_page - file->fil_fudge; - LARGE_INTEGER liOffset; + LARGE_INTEGER liOffset; liOffset.QuadPart = UInt32x32To64((DWORD) page, (DWORD) bcb->bcb_page_size); overlapped->Offset = liOffset.LowPart; diff --git a/src/jrd/pag.cpp b/src/jrd/pag.cpp index d40df23d595..da74b3853a3 100644 --- a/src/jrd/pag.cpp +++ b/src/jrd/pag.cpp @@ -108,6 +108,7 @@ static void add_clump(thread_db* tdbb, USHORT type, USHORT len, const UCHAR* ent static ULONG ensureDiskSpace(thread_db* tdbb, WIN* pip_window, const PageNumber pageNum, ULONG pipUsed); static void find_clump_space(thread_db* tdbb, WIN*, pag**, USHORT, USHORT, const UCHAR*); static bool find_type(thread_db* tdbb, WIN*, pag**, USHORT, USHORT, UCHAR**, const UCHAR**); +static void reopenFilesWithForcedWriteFlag(thread_db* tdbb, bool flag); inline void err_post_if_database_is_readonly(const Database* dbb) { @@ -1676,6 +1677,19 @@ void PAG_set_force_write(thread_db* tdbb, bool flag) err_post_if_database_is_readonly(dbb); + if (dbb->dbb_config->getServerMode() == MODE_SUPER || dbb->dbb_flags & (DBB_new | DBB_creating)) + reopenFilesWithForcedWriteFlag(tdbb, flag); + else + { + if (CCH_exclusive(tdbb, LCK_PW, LCK_NO_WAIT, NULL)) + { + reopenFilesWithForcedWriteFlag(tdbb, flag); + CCH_release_exclusive(tdbb); + } + else + ERR_post(Arg::Gds(isc_forced_write_change_err)); + } + WIN window(HEADER_PAGE_NUMBER); header_page* header = (header_page*) CCH_FETCH(tdbb, &window, LCK_write, pag_header); CCH_MARK_MUST_WRITE(tdbb, &window); @@ -1692,18 +1706,6 @@ void PAG_set_force_write(thread_db* tdbb, bool flag) } CCH_RELEASE(tdbb, &window); - - PageSpace* pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE); - for (jrd_file* file = pageSpace->file; file; file = file->fil_next) { - PIO_force_write(file, flag, dbb->dbb_flags & DBB_no_fs_cache); - } - - for (Shadow* shadow = dbb->dbb_shadow; shadow; shadow = shadow->sdw_next) - { - for (jrd_file* file = shadow->sdw_file; file; file = file->fil_next) { - PIO_force_write(file, flag, dbb->dbb_flags & DBB_no_fs_cache); - } - } } @@ -2118,6 +2120,23 @@ static bool find_type(thread_db* tdbb, } } +static void reopenFilesWithForcedWriteFlag(thread_db* tdbb, bool forceWrite) +{ + Database* dbb = tdbb->getDatabase(); + PageSpace* pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE); + + for (jrd_file* file = pageSpace->file; file; file = file->fil_next) { + PIO_force_write(file, forceWrite, dbb->dbb_flags & DBB_no_fs_cache); + } + + for (Shadow* shadow = dbb->dbb_shadow; shadow; shadow = shadow->sdw_next) + { + for (jrd_file* file = shadow->sdw_file; file; file = file->fil_next) { + PIO_force_write(file, forceWrite, dbb->dbb_flags & DBB_no_fs_cache); + } + } +} + // Class PageSpace starts here