Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix error when changing force write mode #8146

Open
wants to merge 1 commit into
base: v5.0-release
Choose a base branch
from
Open

Fix error when changing force write mode #8146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/include/firebird/impl/msg/jrd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error code is already used in master

1 change: 1 addition & 0 deletions src/include/gen/Firebird.pas
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/jrd/Database.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions src/jrd/nbak.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions src/jrd/os/pio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ class jrd_file : public pool_alloc_rpt<SCHAR, type_fil>
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
};
Expand All @@ -74,8 +75,9 @@ class jrd_file : public pool_alloc_rpt<SCHAR, type_fil>
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
};
Expand Down
27 changes: 21 additions & 6 deletions src/jrd/os/posix/unix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be before EngineCheckout ? At least from logical POV


EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY);

const SLONG size = dbb->dbb_page_size;

for (i = 0; i < IO_RETRY; i++)
Expand Down Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as in PIO_read above


EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY);

const SLONG size = dbb->dbb_page_size;

for (i = 0; i < IO_RETRY; i++)
Expand Down Expand Up @@ -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);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks as not necessary when seek_file() is called from PIO_read() and PIO_write() - the most often cases.
The exception is when caller is PIO_init_data(), so there should be some smart solution.

if (file->fil_desc == -1)
{
unix_error("lseek", file, isc_io_access_err, status_vector);
Expand Down
73 changes: 23 additions & 50 deletions src/jrd/os/win32/winnt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,49 +54,6 @@

#include <windows.h>

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;

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}


Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading