From 7b5b0ca838f891dd8d8bfd74ced00e77f0aae157 Mon Sep 17 00:00:00 2001 From: alexpeshkoff Date: Wed, 30 Dec 2015 15:16:43 +0000 Subject: [PATCH] Fixed CORE-5056: Write-lock of database file is cleared ("W" disappears from output of lsof ) when remote machine obtains DB header running "fbsvcmgr /port:service_mgr action_db_stats " --- lang_helpers/gds_codes.ftn | 2 ++ lang_helpers/gds_codes.pas | 2 ++ src/include/gen/codetext.h | 1 + src/include/gen/iberror.h | 6 ++-- src/include/gen/msgs.h | 1 + src/include/gen/sql_code.h | 1 + src/include/gen/sql_state.h | 1 + src/jrd/os/posix/unix.cpp | 66 ++++++++++++++++++++++--------------- src/msgs/facilities2.sql | 2 +- src/msgs/messages2.sql | 1 + src/msgs/system_errors2.sql | 1 + 11 files changed, 54 insertions(+), 30 deletions(-) diff --git a/lang_helpers/gds_codes.ftn b/lang_helpers/gds_codes.ftn index bfaa001a52..567cbc9ed6 100644 --- a/lang_helpers/gds_codes.ftn +++ b/lang_helpers/gds_codes.ftn @@ -1624,6 +1624,8 @@ C -- PARAMETER (GDS__map_down = 335545105) INTEGER*4 GDS__login_error PARAMETER (GDS__login_error = 335545106) + INTEGER*4 GDS__already_opened + PARAMETER (GDS__already_opened = 335545107) INTEGER*4 GDS__gfix_db_name PARAMETER (GDS__gfix_db_name = 335740929) INTEGER*4 GDS__gfix_invalid_sw diff --git a/lang_helpers/gds_codes.pas b/lang_helpers/gds_codes.pas index cbc74e32e4..45501e24b8 100644 --- a/lang_helpers/gds_codes.pas +++ b/lang_helpers/gds_codes.pas @@ -1619,6 +1619,8 @@ const gds_map_down = 335545105; isc_login_error = 335545106; gds_login_error = 335545106; + isc_already_opened = 335545107; + gds_already_opened = 335545107; isc_gfix_db_name = 335740929; gds_gfix_db_name = 335740929; isc_gfix_invalid_sw = 335740930; diff --git a/src/include/gen/codetext.h b/src/include/gen/codetext.h index c723e19646..715deda6c1 100644 --- a/src/include/gen/codetext.h +++ b/src/include/gen/codetext.h @@ -808,6 +808,7 @@ static const struct { {"invalid_attachment_charset", 335545104}, {"map_down", 335545105}, {"login_error", 335545106}, + {"already_opened", 335545107}, {"gfix_db_name", 335740929}, {"gfix_invalid_sw", 335740930}, {"gfix_incmp_sw", 335740932}, diff --git a/src/include/gen/iberror.h b/src/include/gen/iberror.h index 2230412404..ea4aba3e63 100644 --- a/src/include/gen/iberror.h +++ b/src/include/gen/iberror.h @@ -842,6 +842,7 @@ const ISC_STATUS isc_domain_primary_key_notnull = 335545103L; const ISC_STATUS isc_invalid_attachment_charset = 335545104L; const ISC_STATUS isc_map_down = 335545105L; const ISC_STATUS isc_login_error = 335545106L; +const ISC_STATUS isc_already_opened = 335545107L; const ISC_STATUS isc_gfix_db_name = 335740929L; const ISC_STATUS isc_gfix_invalid_sw = 335740930L; const ISC_STATUS isc_gfix_incmp_sw = 335740932L; @@ -1305,7 +1306,7 @@ const ISC_STATUS isc_trace_switch_user_only = 337182757L; const ISC_STATUS isc_trace_switch_param_miss = 337182758L; const ISC_STATUS isc_trace_param_act_notcompat = 337182759L; const ISC_STATUS isc_trace_mandatory_switch_miss = 337182760L; -const ISC_STATUS isc_err_max = 1249; +const ISC_STATUS isc_err_max = 1250; #else /* c definitions */ @@ -2117,6 +2118,7 @@ const ISC_STATUS isc_err_max = 1249; #define isc_invalid_attachment_charset 335545104L #define isc_map_down 335545105L #define isc_login_error 335545106L +#define isc_already_opened 335545107L #define isc_gfix_db_name 335740929L #define isc_gfix_invalid_sw 335740930L #define isc_gfix_incmp_sw 335740932L @@ -2580,7 +2582,7 @@ const ISC_STATUS isc_err_max = 1249; #define isc_trace_switch_param_miss 337182758L #define isc_trace_param_act_notcompat 337182759L #define isc_trace_mandatory_switch_miss 337182760L -#define isc_err_max 1249 +#define isc_err_max 1250 #endif diff --git a/src/include/gen/msgs.h b/src/include/gen/msgs.h index 53ac46c78a..37cb197dd5 100644 --- a/src/include/gen/msgs.h +++ b/src/include/gen/msgs.h @@ -811,6 +811,7 @@ Data source : @4"}, /* eds_statement */ {335545104, "CHARACTER SET @1 cannot be used as a attachment character set"}, /* invalid_attachment_charset */ {335545105, "Some database(s) were shutdown when trying to read mapping data"}, /* map_down */ {335545106, "Error occurred during login, please check server firebird.log for details"}, /* login_error */ + {335545107, "Database already opened with engine instance, incompatible with current"}, /* already_opened */ {335740929, "data base file name (@1) already given"}, /* gfix_db_name */ {335740930, "invalid switch @1"}, /* gfix_invalid_sw */ {335740932, "incompatible switch combination"}, /* gfix_incmp_sw */ diff --git a/src/include/gen/sql_code.h b/src/include/gen/sql_code.h index ddc6515a95..23ea0c7a79 100644 --- a/src/include/gen/sql_code.h +++ b/src/include/gen/sql_code.h @@ -807,6 +807,7 @@ static const struct { {335545104, -204}, /* 784 invalid_attachment_charset */ {335545105, -901}, /* 785 map_down */ {335545106, -902}, /* 786 login_error */ + {335545107, -902}, /* 787 already_opened */ {335740929, -901}, /* 1 gfix_db_name */ {335740930, -901}, /* 2 gfix_invalid_sw */ {335740932, -901}, /* 4 gfix_incmp_sw */ diff --git a/src/include/gen/sql_state.h b/src/include/gen/sql_state.h index 397b31418e..a2d5cf89ae 100644 --- a/src/include/gen/sql_state.h +++ b/src/include/gen/sql_state.h @@ -807,6 +807,7 @@ static const struct { {335545104, "2C000"}, // 784 invalid_attachment_charset {335545105, "08004"}, // 785 map_down {335545106, "08006"}, // 786 login_error + {335545107, "08006"}, // 787 already_opened {335740929, "00000"}, // 1 gfix_db_name {335740930, "00000"}, // 2 gfix_invalid_sw {335740932, "00000"}, // 4 gfix_incmp_sw diff --git a/src/jrd/os/posix/unix.cpp b/src/jrd/os/posix/unix.cpp index 6596070a6d..6d91c06031 100644 --- a/src/jrd/os/posix/unix.cpp +++ b/src/jrd/os/posix/unix.cpp @@ -112,7 +112,8 @@ static const mode_t MASK = 0660; #define FCNTL_BROKEN 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); -static bool lockDatabaseFile(int desc, const bool shareMode, const bool temporary = false); +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); #if !(defined HAVE_PREAD && defined HAVE_PWRITE) static SLONG pread(int, SCHAR*, SLONG, SLONG); @@ -217,7 +218,7 @@ jrd_file* PIO_create(thread_db* tdbb, const PathName& file_name, Database* const dbb = tdbb->getDatabase(); - const int desc = os_utils::open(file_name.c_str(), flag, 0666); + int desc = os_utils::open(file_name.c_str(), flag, 0666); if (desc == -1) { ERR_post(Arg::Gds(isc_io_error) << Arg::Str("open O_CREAT") << Arg::Str(file_name) << @@ -225,15 +226,7 @@ jrd_file* PIO_create(thread_db* tdbb, const PathName& file_name, } const bool shareMode = dbb->dbb_config->getServerMode() != MODE_SUPER; - if (!lockDatabaseFile(desc, shareMode, temporary)) - { - int lockErrno = errno; - close(desc); - // error when locking file almost always means it's locked by someone else - // therefore do not remove file here (contrary to chmod error) - ERR_post(Arg::Gds(isc_io_error) << Arg::Str("lock") << Arg::Str(file_name) << - Arg::Gds(isc_io_create_err) << Arg::Unix(lockErrno)); - } + lockDatabaseFile(desc, shareMode, temporary, file_name.c_str(), isc_io_create_err); #ifdef HAVE_FCHMOD if (fchmod(desc, MASK) < 0) @@ -440,10 +433,7 @@ void PIO_force_write(jrd_file* file, const bool forcedWrites, const bool notUseF unix_error("re open() for SYNC/DIRECT", file, isc_io_open_err); } - if (!lockDatabaseFile(file->fil_desc, file->fil_flags & FIL_sh_write)) - { - unix_error("lock", file, isc_io_open_err); - } + lockDatabaseFile(file->fil_desc, file->fil_flags & FIL_sh_write, false, file->fil_string, isc_io_open_err); #endif //FCNTL_BROKEN #ifdef SOLARIS @@ -677,11 +667,7 @@ jrd_file* PIO_open(thread_db* tdbb, } const bool shareMode = dbb->dbb_config->getServerMode() != MODE_SUPER; - if (!lockDatabaseFile(desc, shareMode || readOnly)) - { - ERR_post(Arg::Gds(isc_io_error) << Arg::Str("lock") << Arg::Str(file_name) << - Arg::Gds(isc_io_open_err) << Arg::Unix(errno)); - } + lockDatabaseFile(desc, shareMode || readOnly, false, ptr, isc_io_open_err); // posix_fadvise(desc, 0, 0, POSIX_FADV_RANDOM); @@ -942,15 +928,41 @@ static jrd_file* setup_file(Database* dbb, } -static bool lockDatabaseFile(int desc, const bool share, const bool temporary) +static void lockDatabaseFile(int& desc, const bool share, const bool temporary, + const char* fileName, ISC_STATUS operation) { - struct flock lck; - lck.l_type = (!temporary && share) ? F_RDLCK : F_WRLCK; - lck.l_whence = SEEK_SET; - lck.l_start = 0; - lck.l_len = 0; + bool shared = (!temporary) && share; + bool busy = false; - return fcntl(desc, F_SETLK, &lck) == 0; + do + { +#ifndef HAVE_FLOCK + struct flock lck; + lck.l_type = shared ? F_RDLCK : F_WRLCK; + lck.l_whence = SEEK_SET; + lck.l_start = 0; + lck.l_len = 0; + + if (fcntl(desc, F_SETLK, &lck) == 0) + return; + busy = (errno == EACCES) || (errno == EAGAIN); +#else + if (flock(desc, (shared ? LOCK_SH : LOCK_EX) | LOCK_NB) == 0) + return; + busy = (errno == EWOULDBLOCK); +#endif + } + while (errno == EINTR); + + maybeCloseFile(desc); + + Arg::Gds err(isc_io_error); + err << "lock" << fileName; + if (busy) + err << Arg::Gds(isc_already_opened); + else + err << Arg::Gds(operation) << Arg::Unix(errno); + ERR_post(err); } diff --git a/src/msgs/facilities2.sql b/src/msgs/facilities2.sql index e78dda1a17..410a8f5728 100644 --- a/src/msgs/facilities2.sql +++ b/src/msgs/facilities2.sql @@ -1,7 +1,7 @@ /* MAX_NUMBER is the next number to be used, always one more than the highest message number. */ set bulk_insert INSERT INTO FACILITIES (LAST_CHANGE, FACILITY, FAC_CODE, MAX_NUMBER) VALUES (?, ?, ?, ?); -- -('2015-12-22 20:33:22', 'JRD', 0, 787) +('2015-12-30 17:52:25', 'JRD', 0, 788) ('2015-03-17 18:33:00', 'QLI', 1, 533) ('2015-01-07 18:01:51', 'GFIX', 3, 134) ('1996-11-07 13:39:40', 'GPRE', 4, 1) diff --git a/src/msgs/messages2.sql b/src/msgs/messages2.sql index c3506d5c56..787ca706a5 100644 --- a/src/msgs/messages2.sql +++ b/src/msgs/messages2.sql @@ -894,6 +894,7 @@ Data source : @4', NULL, NULL) ('invalid_attachment_charset', NULL, NULL, NULL, 0, 784, NULL, 'CHARACTER SET @1 cannot be used as a attachment character set', NULL, NULL); ('map_down', NULL, 'Mapping.cpp', NULL, 0, 785, NULL, 'Some database(s) were shutdown when trying to read mapping data', NULL, NULL); ('login_error', NULL, 'server.cpp', NULL, 0, 786, NULL, 'Error occurred during login, please check server firebird.log for details', NULL, NULL); +('already_opened', 'lockDatabaseFile', 'unix.cpp', NULL, 0, 787, NULL, 'Database already opened with engine instance, incompatible with current', NULL, NULL); -- QLI (NULL, NULL, NULL, NULL, 1, 0, NULL, 'expected type', NULL, NULL); (NULL, NULL, NULL, NULL, 1, 1, NULL, 'bad block type', NULL, NULL); diff --git a/src/msgs/system_errors2.sql b/src/msgs/system_errors2.sql index d8a3eeb6c6..53f2cbef00 100644 --- a/src/msgs/system_errors2.sql +++ b/src/msgs/system_errors2.sql @@ -793,6 +793,7 @@ set bulk_insert INSERT INTO SYSTEM_ERRORS (SQL_CODE, SQL_CLASS, SQL_SUBCLASS, FA (-204, '2C', '000', 0, 784, 'invalid_attachment_charset', NULL, NULL) (-901, '08', '004', 0, 785, 'map_down', NULL, NULL) (-902, '08', '006', 0, 786, 'login_error', NULL, NULL) +(-902, '08', '006', 0, 787, 'already_opened', NULL, NULL) -- GFIX (-901, '00', '000', 3, 1, 'gfix_db_name', NULL, NULL) (-901, '00', '000', 3, 2, 'gfix_invalid_sw', NULL, NULL)