Skip to content

Commit

Permalink
Fixed bug CORE-5528 : internal Firebird consistency check (limbo impo…
Browse files Browse the repository at this point in the history
…ssible (184), file: vio.cpp line: 2379)
  • Loading branch information
hvlad committed Apr 28, 2017
1 parent 40b5357 commit 6c5a2ae
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/jrd/vio.cpp
Expand Up @@ -2331,6 +2331,11 @@ bool VIO_get_current(thread_db* tdbb,
VIO_data(tdbb, rpb, pool);
return true;

case tra_limbo:
if (!(transaction->tra_flags & TRA_ignore_limbo))
ERR_post(Arg::Gds(isc_rec_in_limbo) << Arg::Num(rpb->rpb_transaction_nr));
// fall thru

case tra_active:
// clear lock error from status vector
fb_utils::init_status(tdbb->tdbb_status_vector);
Expand Down Expand Up @@ -2375,9 +2380,6 @@ bool VIO_get_current(thread_db* tdbb,
}
break;

case tra_limbo:
BUGCHECK(184); // limbo impossible
break;

default:
fb_assert(false);
Expand Down Expand Up @@ -5811,7 +5813,7 @@ static int prepare_update( thread_db* tdbb,
return PREPARE_LOCKERR;

case tra_limbo:
ERR_post(Arg::Gds(isc_deadlock) << Arg::Gds(isc_trainlim));
ERR_post(Arg::Gds(isc_trainlim) << Arg::Gds(isc_rec_in_limbo) << Arg::Num(rpb->rpb_transaction_nr));

This comment has been minimized.

Copy link
@dyemanov

dyemanov Apr 28, 2017

Member

Does this error mean something different than the above one? In particular, shouldn't they both throw the same status vector, either (isc_rec_in_limbo) or (isc_trainlim + isc_rec_in_limbo)?

This comment has been minimized.

Copy link
@hvlad

hvlad Apr 28, 2017

Author Member

Not sure i completely got you. Intentions was:

  • remove (absolutely wrong) isc_deadlock
  • leave isc_trainlim for backward compatibility
  • add info about conflicting transaction

This comment has been minimized.

Copy link
@dyemanov

dyemanov Apr 28, 2017

Member

My point was about #2 from your list. Should isc_trainlim be really used in line 5814/5816, in your opinion? If it's incorrect / redundant but preserved just for backward compatibility, then it's ok. If its usage is valid, then shouldn't the first place also raise isc_trainlim before isc_rec_in_limbo, just to be consistent?

This comment has been minimized.

Copy link
@hvlad

hvlad Apr 28, 2017

Author Member

I consider isc_rec_in_limbo more informative and use it in patch for master.
Here i leave it for backward compatibility only, but probably i am too conservative :)

This comment has been minimized.

Copy link
@hvlad

hvlad Apr 28, 2017

Author Member

As for the usage at first place (VIO_get_current) - it works in the same way as VIO_chase

case tra_dead:
break;
Expand Down

0 comments on commit 6c5a2ae

Please sign in to comment.