diff options
| -rw-r--r-- | Bugzilla/Constants.pm | 5 | ||||
| -rw-r--r-- | Bugzilla/DB.pm | 116 | ||||
| -rw-r--r-- | Bugzilla/DB/Mysql.pm | 38 | ||||
| -rw-r--r-- | Bugzilla/Error.pm | 15 | ||||
| -rwxr-xr-x | process_bug.cgi | 39 | ||||
| -rw-r--r-- | template/en/default/global/code-error.html.tmpl | 8 |
6 files changed, 13 insertions, 208 deletions
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index ee064bb85..31427bc82 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -91,7 +91,6 @@ use File::Basename; CMT_POPULAR_VOTES CMT_MOVED_TO - UNLOCK_ABORT THROW_ERROR RELATIONSHIPS @@ -269,10 +268,6 @@ use constant CMT_HAS_DUPE => 2; use constant CMT_POPULAR_VOTES => 3; use constant CMT_MOVED_TO => 4; -# used by Bugzilla::DB to indicate that tables are being unlocked -# because of error -use constant UNLOCK_ABORT => 1; - # Determine whether a validation routine should return 0 or throw # an error when the validation fails. use constant THROW_ERROR => 1; diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index 2a71bcd75..4a0303287 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -273,8 +273,7 @@ EOT # List of abstract methods we are checking the derived class implements our @_abstract_methods = qw(REQUIRED_VERSION PROGRAM_NAME DBD_VERSION new sql_regexp sql_not_regexp sql_limit sql_to_days - sql_date_format sql_interval - bz_lock_tables bz_unlock_tables); + sql_date_format sql_interval); # This overridden import method will check implementation of inherited classes # for missing implementation of abstract methods @@ -301,62 +300,6 @@ sub import { $Exporter::ExportLevel-- if $is_exporter; } -sub bz_lock_tables { - my ($self, @tables) = @_; - - my $list = join(', ', @tables); - # Check first if there was no lock before - if ($self->{private_bz_tables_locked}) { - ThrowCodeError("already_locked", { current => $self->{private_bz_tables_locked}, - new => $list }); - } else { - my %read_tables; - my %write_tables; - foreach my $table (@tables) { - $table =~ /^([\d\w]+)([\s]+AS[\s]+[\d\w]+)?[\s]+(WRITE|READ)$/i; - my $table_name = $1; - if ($3 =~ /READ/i) { - if (!exists $read_tables{$table_name}) { - $read_tables{$table_name} = undef; - } - } - else { - if (!exists $write_tables{$table_name}) { - $write_tables{$table_name} = undef; - } - } - } - - # Begin Transaction - $self->bz_start_transaction(); - - Bugzilla->dbh->do('LOCK TABLE ' . join(', ', keys %read_tables) . - ' IN ROW SHARE MODE') if keys %read_tables; - Bugzilla->dbh->do('LOCK TABLE ' . join(', ', keys %write_tables) . - ' IN ROW EXCLUSIVE MODE') if keys %write_tables; - $self->{private_bz_tables_locked} = $list; - } -} - -sub bz_unlock_tables { - my ($self, $abort) = @_; - - # Check first if there was previous matching lock - if (!$self->{private_bz_tables_locked}) { - # Abort is allowed even without previous lock for error handling - return if $abort; - ThrowCodeError("no_matching_lock"); - } else { - $self->{private_bz_tables_locked} = ""; - # End transaction, tables will be unlocked automatically - if ($abort) { - $self->bz_rollback_transaction(); - } else { - $self->bz_commit_transaction(); - } - } -} - sub sql_istrcmp { my ($self, $left, $right, $op) = @_; $op ||= "="; @@ -1949,63 +1892,6 @@ Formatted SQL for the C<IN> operator. =back -=item C<bz_lock_tables> - -=over - -=item B<Description> - -Performs a table lock operation on specified tables. If the underlying -database supports transactions, it should also implicitly start a new -transaction. - -Abstract method, should be overridden by database specific code. - -=item B<Params> - -=over - -=item C<@tables> - list of names of tables to lock in MySQL -notation (ex. 'bugs AS bugs2 READ', 'logincookies WRITE') - -=back - -=item B<Returns> (nothing) - -=back - -=item C<bz_unlock_tables> - -=over - -=item B<Description> - -Performs a table unlock operation. - -If the underlying database supports transactions, it should also implicitly -commit or rollback the transaction. - -Also, this function should allow to be called with the abort flag -set even without locking tables first without raising an error -to simplify error handling. - -Abstract method, should be overridden by database specific code. - -=item B<Params> - -=over - -=item C<$abort> - C<UNLOCK_ABORT> if the operation on locked tables -failed (if transactions are supported, the action will be rolled -back). No param if the operation succeeded. This is only used by -L<Bugzilla::Error/throw_error>. - -=back - -=item B<Returns> (none) - -=back - =back diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm index 9e0d25277..720c8ff42 100644 --- a/Bugzilla/DB/Mysql.pm +++ b/Bugzilla/DB/Mysql.pm @@ -200,44 +200,6 @@ sub sql_group_by { } -sub bz_lock_tables { - my ($self, @tables) = @_; - - my $list = join(', ', @tables); - # Check first if there was no lock before - if ($self->{private_bz_tables_locked}) { - ThrowCodeError("already_locked", { current => $self->{private_bz_tables_locked}, - new => $list }); - } else { - $self->bz_start_transaction(); - $self->do('LOCK TABLE ' . $list); - $self->{private_bz_tables_locked} = $list; - } -} - -sub bz_unlock_tables { - my ($self, $abort) = @_; - - if ($self->bz_in_transaction) { - if ($abort) { - $self->bz_rollback_transaction(); - } - else { - $self->bz_commit_transaction(); - } - } - - # Check first if there was previous matching lock - if (!$self->{private_bz_tables_locked}) { - # Abort is allowed even without previous lock for error handling - return if $abort; - ThrowCodeError("no_matching_lock"); - } else { - $self->do("UNLOCK TABLES"); - $self->{private_bz_tables_locked} = ""; - } -} - sub _bz_get_initial_schema { my ($self) = @_; return $self->_bz_build_schema_from_disk(); diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm index 18f9aae54..3e5688e2a 100644 --- a/Bugzilla/Error.pm +++ b/Bugzilla/Error.pm @@ -46,16 +46,15 @@ sub _in_eval { sub _throw_error { my ($name, $error, $vars) = @_; - + my $dbh = Bugzilla->dbh; $vars ||= {}; $vars->{error} = $error; - # Make sure any locked tables are unlocked - # and the transaction is rolled back (if supported) - # If we are within an eval(), do not unlock tables as we are + # Make sure any transaction is rolled back (if supported). + # If we are within an eval(), do not roll back transactions as we are # eval'uating some test on purpose. - Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT) unless _in_eval(); + $dbh->bz_rollback_transaction() if ($dbh->bz_in_transaction() && !_in_eval()); my $datadir = bz_locations()->{'datadir'}; # If a writable $datadir/errorlog exists, log error details there. @@ -124,10 +123,10 @@ sub ThrowCodeError { sub ThrowTemplateError { my ($template_err) = @_; + my $dbh = Bugzilla->dbh; - # Make sure any locked tables are unlocked - # and the transaction is rolled back (if supported) - Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT); + # Make sure the transaction is rolled back (if supported). + $dbh->bz_rollback_transaction() if $dbh->bz_in_transaction(); my $vars = {}; if (Bugzilla->error_mode == ERROR_MODE_DIE) { diff --git a/process_bug.cgi b/process_bug.cgi index 912440ce2..8eb7aaf33 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -398,17 +398,7 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) { $user->is_mover || ThrowUserError("auth_failure", {action => 'move', object => 'bugs'}); - my @multi_select_locks = map {'bug_' . $_->name . " WRITE"} - Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT, - obsolete => 0 }); - - $dbh->bz_lock_tables('bugs WRITE', 'bugs_activity WRITE', 'duplicates WRITE', - 'longdescs WRITE', 'profiles READ', 'groups READ', - 'bug_group_map READ', 'group_group_map READ', - 'user_group_map READ', 'classifications READ', - 'products READ', 'components READ', 'votes READ', - 'cc READ', 'fielddefs READ', 'bug_status READ', - 'status_workflow READ', 'resolution READ', @multi_select_locks); + $dbh->bz_start_transaction(); # First update all moved bugs. foreach my $bug (@bug_objects) { @@ -430,7 +420,7 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) { $bug->_clear_dup_id; } $_->update() foreach @bug_objects; - $dbh->bz_unlock_tables(); + $dbh->bz_commit_transaction(); # Now send emails. foreach my $bug (@bug_objects) { @@ -510,28 +500,9 @@ foreach my $b (@bug_objects) { # Do Actual Database Updates # ############################## foreach my $bug (@bug_objects) { - my $write = "WRITE"; # Might want to make a param to control - # whether we do LOW_PRIORITY ... - - my @multi_select_locks = map {'bug_' . $_->name . " $write"} - Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT, - obsolete => 0 }); - - $dbh->bz_lock_tables("bugs $write", "bugs_activity $write", "cc $write", - "profiles READ", "dependencies $write", "votes $write", - "products READ", "components READ", "milestones READ", - "keywords $write", "longdescs $write", "fielddefs READ", - "bug_group_map $write", "flags $write", "duplicates $write", - "user_group_map READ", "group_group_map READ", "flagtypes READ", - "flaginclusions AS i READ", "flagexclusions AS e READ", - "keyworddefs READ", "groups READ", "attachments READ", - "bug_status READ", "group_control_map AS oldcontrolmap READ", - "group_control_map AS newcontrolmap READ", - "group_control_map READ", "email_setting READ", - "classifications READ", @multi_select_locks); - - my $timestamp = $dbh->selectrow_array(q{SELECT NOW()}); + $dbh->bz_start_transaction(); + my $timestamp = $dbh->selectrow_array(q{SELECT NOW()}); my $changes = $bug->update($timestamp); my %notify_deps; @@ -581,7 +552,7 @@ foreach my $bug (@bug_objects) { # Set and update flags. Bugzilla::Flag::process($bug, undef, $timestamp, $cgi, $vars); - $dbh->bz_unlock_tables(); + $dbh->bz_commit_transaction(); ############### # Send Emails # diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 1e5581e13..7e0e1cd70 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -416,14 +416,6 @@ [% ELSIF error == "not_in_transaction" %] Attempted to end transaction without starting one first. - [% ELSIF error == "already_locked" %] - Attempted to lock a table without releasing previous lock first: - <p>Tables already locked:<br>[% current FILTER html %] - <p>Tables requesting locking:<br>[% new FILTER html %] - - [% ELSIF error == "no_matching_lock" %] - Attempted to unlock tables without locking them first. - [% ELSIF error == "comma_operator_deprecated" %] [% title = "SQL query generator internal error" %] There is an internal error in the SQL query generation code, |