From 62c900feb2d38d6f2c499e6dafd9b8352e58ccfd Mon Sep 17 00:00:00 2001 From: Ilya Eremin Date: Thu, 18 Aug 2022 12:52:17 +0300 Subject: [PATCH 1/4] Print a function name in error messages in cases when a file descriptor is invalid (like it's done in PIO_header) for better diagnostics --- src/jrd/os/posix/unix.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/jrd/os/posix/unix.cpp b/src/jrd/os/posix/unix.cpp index 7a146367750..0da4a095a97 100644 --- a/src/jrd/os/posix/unix.cpp +++ b/src/jrd/os/posix/unix.cpp @@ -480,7 +480,7 @@ ULONG PIO_get_number_of_pages(const jrd_file* file, const USHORT pagesize) **************************************/ if (file->fil_desc == -1) - unix_error("fstat", file, isc_io_access_err); + unix_error("PIO_get_number_of_pages", file, isc_io_access_err); struct STAT statistics; if (os_utils::fstat(file->fil_desc, &statistics)) @@ -752,7 +752,7 @@ bool PIO_read(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, FB_UINT64 offset; if (file->fil_desc == -1) - return unix_error("read", file, isc_io_read_err, status_vector); + return unix_error("PIO_read", file, isc_io_read_err, status_vector); Database* const dbb = tdbb->getDatabase(); @@ -804,7 +804,7 @@ bool PIO_write(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, FB_UINT64 offset; if (file->fil_desc == -1) - return unix_error("write", file, isc_io_write_err, status_vector); + return unix_error("PIO_write", file, isc_io_write_err, status_vector); Database* const dbb = tdbb->getDatabase(); @@ -860,7 +860,7 @@ static jrd_file* seek_file(jrd_file* file, BufferDesc* bdb, FB_UINT64* offset, if (file->fil_desc == -1) { - unix_error("lseek", file, isc_io_access_err, status_vector); + unix_error("seek_file", file, isc_io_access_err, status_vector); return 0; } From 556c3fd4372638c555d5c7c753447ba38102e379 Mon Sep 17 00:00:00 2001 From: Ilya Eremin Date: Thu, 18 Aug 2022 12:52:47 +0300 Subject: [PATCH 2/4] Put the error message into firebird.log before BUGCHECK actions because there is always a risk of crashing --- src/jrd/err.cpp | 51 ++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/src/jrd/err.cpp b/src/jrd/err.cpp index dd4bd9585c2..4c83ca6c1f4 100644 --- a/src/jrd/err.cpp +++ b/src/jrd/err.cpp @@ -53,7 +53,7 @@ using namespace Firebird; //#define JRD_FAILURE_UNKNOWN "" // Used when buffer fails -static void internal_error(ISC_STATUS status, int number, const TEXT* file = NULL, int line = 0); +static void get_internal_error_msg(TEXT* errmsg_buf, size_t errmsg_buf_size, int number, const TEXT* file = NULL, int line = 0); static void post_nothrow(const unsigned lenToAdd, const ISC_STATUS* toAdd, FbStatusVector* statusVector); @@ -69,13 +69,10 @@ void ERR_bugcheck(int number, const TEXT* file, int line) * Things seem to be going poorly today. * **************************************/ - thread_db* const tdbb = JRD_get_thread_data(); - Database* const dbb = tdbb->getDatabase(); - - dbb->dbb_flags |= DBB_bugcheck; - CCH_shutdown(tdbb); + TEXT errmsg[MAX_ERRMSG_LEN + 1]; + get_internal_error_msg(errmsg, sizeof(errmsg), number, file, line); - internal_error(isc_bug_check, number, file, line); + ERR_bugcheck_msg(errmsg); } @@ -94,10 +91,23 @@ void ERR_bugcheck_msg(const TEXT* msg) thread_db* const tdbb = JRD_get_thread_data(); Database* const dbb = tdbb->getDatabase(); + Arg::StatusVector status_vector; + status_vector << Arg::Gds(isc_bug_check) << Arg::Str(msg); + FbLocalStatus status; + status_vector.copyTo(&status); + + // It's important to put the message into the log before any + // further actions because there is always a risk of crashing. + iscDbLogStatus(dbb->dbb_filename.nullStr(), &status); + dbb->dbb_flags |= DBB_bugcheck; CCH_shutdown(tdbb); - ERR_post(Arg::Gds(isc_bug_check) << Arg::Str(msg)); + if (Config::getBugcheckAbort()) + abort(); + + ERR_post_nothrow(status_vector); + status_exception::raise(tdbb->tdbb_status_vector); } @@ -117,7 +127,10 @@ void ERR_soft_bugcheck(int number, const TEXT* file, int line) **************************************/ fb_assert(false); - internal_error(isc_bug_check, number, file, line); + TEXT errmsg[MAX_ERRMSG_LEN + 1]; + get_internal_error_msg(errmsg, sizeof(errmsg), number, file, line); + + ERR_post(Arg::Gds(isc_bug_check) << Arg::Str(errmsg)); } @@ -133,8 +146,10 @@ void ERR_corrupt(int number) * Things seem to be going poorly today. * **************************************/ + TEXT errmsg[MAX_ERRMSG_LEN + 1]; + get_internal_error_msg(errmsg, sizeof(errmsg), number); - internal_error(isc_db_corrupt, number); + ERR_post(Arg::Gds(isc_db_corrupt) << Arg::Str(errmsg)); } @@ -413,7 +428,7 @@ void ERR_build_status(FbStatusVector* status_vector, const Arg::StatusVector& v) } -static void internal_error(ISC_STATUS status, int number, const TEXT* file, int line) +static void get_internal_error_msg(TEXT* errmsg_buf, size_t errmsg_buf_size, int number, const TEXT* file, int line) { /************************************** * @@ -425,12 +440,10 @@ static void internal_error(ISC_STATUS status, int number, const TEXT* file, int * Things seem to be going poorly today. * **************************************/ - TEXT errmsg[MAX_ERRMSG_LEN + 1]; + if (gds__msg_lookup(0, FB_IMPL_MSG_FACILITY_JRD_BUGCHK, number, errmsg_buf_size, errmsg_buf, NULL) < 1) + strcpy(errmsg_buf, "Internal error code"); - if (gds__msg_lookup(0, FB_IMPL_MSG_FACILITY_JRD_BUGCHK, number, sizeof(errmsg), errmsg, NULL) < 1) - strcpy(errmsg, "Internal error code"); - - const size_t len = strlen(errmsg); + const size_t len = strlen(errmsg_buf); if (file) { @@ -444,12 +457,10 @@ static void internal_error(ISC_STATUS status, int number, const TEXT* file, int break; } } - fb_utils::snprintf(errmsg + len, sizeof(errmsg) - len, + fb_utils::snprintf(errmsg_buf + len, errmsg_buf_size - len, " (%d), file: %s line: %d", number, ptr, line); } else { - fb_utils::snprintf(errmsg + len, sizeof(errmsg) - len, " (%d)", number); + fb_utils::snprintf(errmsg_buf + len, errmsg_buf_size - len, " (%d)", number); } - - ERR_post(Arg::Gds(status) << Arg::Str(errmsg)); } From e4c084f2b32d9878d60ae661add97720956a758a Mon Sep 17 00:00:00 2001 From: Ilya Eremin Date: Thu, 18 Aug 2022 12:54:28 +0300 Subject: [PATCH 3/4] Do not print errno in error messages in cases when a file descriptor is invalid and I/O functions were not called before --- src/jrd/os/posix/unix.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/jrd/os/posix/unix.cpp b/src/jrd/os/posix/unix.cpp index 0da4a095a97..9c11242bfc8 100644 --- a/src/jrd/os/posix/unix.cpp +++ b/src/jrd/os/posix/unix.cpp @@ -127,7 +127,7 @@ static jrd_file* seek_file(jrd_file*, BufferDesc*, FB_UINT64*, FbStatusVector*); static jrd_file* setup_file(Database*, const PathName&, const int, const bool, const bool, const bool); static void lockDatabaseFile(int& desc, const bool shareMode, const bool temporary, const char* fileName, ISC_STATUS operation); -static bool unix_error(const TEXT*, const jrd_file*, ISC_STATUS, FbStatusVector* = NULL); +static bool unix_error(const TEXT*, const jrd_file*, ISC_STATUS, FbStatusVector* = NULL, bool print_errno = true); static bool block_size_error(const jrd_file*, off_t, FbStatusVector* = NULL); #if !(defined HAVE_PREAD && defined HAVE_PWRITE) static SLONG pread(int, SCHAR*, SLONG, SLONG); @@ -480,7 +480,7 @@ ULONG PIO_get_number_of_pages(const jrd_file* file, const USHORT pagesize) **************************************/ if (file->fil_desc == -1) - unix_error("PIO_get_number_of_pages", file, isc_io_access_err); + unix_error("PIO_get_number_of_pages", file, isc_io_access_err, NULL, false); struct STAT statistics; if (os_utils::fstat(file->fil_desc, &statistics)) @@ -546,7 +546,7 @@ void PIO_header(thread_db* tdbb, UCHAR* address, int length) jrd_file* file = pageSpace->file; if (file->fil_desc == -1) - unix_error("PIO_header", file, isc_io_read_err); + unix_error("PIO_header", file, isc_io_read_err, NULL, false); for (i = 0; i < IO_RETRY; i++) { @@ -752,7 +752,7 @@ bool PIO_read(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, FB_UINT64 offset; if (file->fil_desc == -1) - return unix_error("PIO_read", file, isc_io_read_err, status_vector); + return unix_error("PIO_read", file, isc_io_read_err, status_vector, false); Database* const dbb = tdbb->getDatabase(); @@ -804,7 +804,7 @@ bool PIO_write(thread_db* tdbb, jrd_file* file, BufferDesc* bdb, Ods::pag* page, FB_UINT64 offset; if (file->fil_desc == -1) - return unix_error("PIO_write", file, isc_io_write_err, status_vector); + return unix_error("PIO_write", file, isc_io_write_err, status_vector, false); Database* const dbb = tdbb->getDatabase(); @@ -860,7 +860,7 @@ static jrd_file* seek_file(jrd_file* file, BufferDesc* bdb, FB_UINT64* offset, if (file->fil_desc == -1) { - unix_error("seek_file", file, isc_io_access_err, status_vector); + unix_error("seek_file", file, isc_io_access_err, status_vector, false); return 0; } @@ -1015,7 +1015,7 @@ static void lockDatabaseFile(int& desc, const bool share, const bool temporary, static bool unix_error(const TEXT* string, const jrd_file* file, ISC_STATUS operation, - FbStatusVector* status_vector) + FbStatusVector* status_vector, bool print_errno) { /************************************** * @@ -1030,7 +1030,10 @@ static bool unix_error(const TEXT* string, **************************************/ Arg::Gds err(isc_io_error); err << string << file->fil_string << - Arg::Gds(operation) << Arg::Unix(errno); + Arg::Gds(operation); + + if (print_errno) + err << Arg::Unix(errno); if (!status_vector) ERR_post(err); From d42db219ed692f1949c4b0e7a77dd2c1e0eba5b4 Mon Sep 17 00:00:00 2001 From: Ilya Eremin Date: Tue, 27 Dec 2022 16:37:11 +0300 Subject: [PATCH 4/4] Do not reset buffer flags in CCH_shutdown when it's called from ERR_bugcheck_msg because otherwise a precedence relationship can be ruined for other threads that may still be active --- src/jrd/cch.cpp | 4 ++-- src/jrd/cch_proto.h | 2 +- src/jrd/err.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/jrd/cch.cpp b/src/jrd/cch.cpp index ddf3bb7bd07..9e5d1ffa99f 100644 --- a/src/jrd/cch.cpp +++ b/src/jrd/cch.cpp @@ -2094,7 +2094,7 @@ bool CCH_rollover_to_shadow(thread_db* tdbb, Database* dbb, jrd_file* file, cons } -void CCH_shutdown(thread_db* tdbb) +void CCH_shutdown(thread_db* tdbb, bool bugcheck) { /************************************** * @@ -2144,7 +2144,7 @@ void CCH_shutdown(thread_db* tdbb) bcb_repeat* tail = bcb->bcb_rpt; const bcb_repeat* const end = tail + bcb->bcb_count; - if (tail && tail->bcb_bdb) + if (tail && tail->bcb_bdb && !bugcheck) { try { diff --git a/src/jrd/cch_proto.h b/src/jrd/cch_proto.h index c04f204b42b..ee60159d245 100644 --- a/src/jrd/cch_proto.h +++ b/src/jrd/cch_proto.h @@ -70,7 +70,7 @@ bool CCH_prefetch_pages(Jrd::thread_db*); void CCH_release(Jrd::thread_db*, Jrd::win*, const bool); void CCH_release_exclusive(Jrd::thread_db*); bool CCH_rollover_to_shadow(Jrd::thread_db* tdbb, Jrd::Database* dbb, Jrd::jrd_file*, const bool); -void CCH_shutdown(Jrd::thread_db*); +void CCH_shutdown(Jrd::thread_db*, bool bugcheck = false); void CCH_unwind(Jrd::thread_db*, const bool); bool CCH_validate(Jrd::win*); void CCH_flush_ast(Jrd::thread_db*); diff --git a/src/jrd/err.cpp b/src/jrd/err.cpp index 4c83ca6c1f4..d96e7ad0be0 100644 --- a/src/jrd/err.cpp +++ b/src/jrd/err.cpp @@ -101,7 +101,7 @@ void ERR_bugcheck_msg(const TEXT* msg) iscDbLogStatus(dbb->dbb_filename.nullStr(), &status); dbb->dbb_flags |= DBB_bugcheck; - CCH_shutdown(tdbb); + CCH_shutdown(tdbb, true); if (Config::getBugcheckAbort()) abort();