Closed Bug 189388 Opened 19 years ago Closed 17 years ago
Add run-time check for nested locks
When changing bug owners, editcomponents.cgi may call functions withing a section where the databases are locked that, in turn, try to lock the tables. That can cause a deadlock. This should not be permitted and needs to be scrubbed. This needs a lock counter to turn attempts to lock while already locked into exceptions.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.18
No generic functions should ever LOCK tables. Not only because of this, but because the unlock would then unlock all of the locks. Which particular functions have locks?
It looks like the cause for this was already fixed since I pulled the code for my live site. It used to be that DBnameToId was calling derivegroups and populating the user data structure. The problem was that it was then being called from within a locked section in editcomponents.cgi Even that was not enough to surface the problem because mysql treats the attempt to lock while already locked as a replacement of one set of locks with another. So, this required the code to get into a state where it cannot get the new lock and, therefore, held onto the old lock in order to cause the deadlock. The only scenario I can think of that would let that deadlock is if ... editcomponents locks set A and calls something that tries to lock set B meanwhile, another thread that already has something in set B locked is trying to lock something in set A. It is kind of silly that MySQL doesn't consider a LOCK attempt when already locked to be a run-time error. What we should do is store caller() when Bugzilla::DB is asked to LOCK and clear it when it is asked to UNLOCK. If it is already set when we are asked to LOCK, throw an exception [dumping caller() andthe prior caller()] Incidentally, we really should find away to dump caller() when there is an SQL error as well.
Severity: major → enhancement
Priority: P2 → P4
Summary: editcomponents.cgi can deadlock (maybe others) → Add run-time check for nested locks
Well, I dislike derivegroups, as you know :) postgres tests for deadlocks, and mysql claims to avoid them, although I don't know if that covers LOCK over multiple statements. We don't want to check them, we want to avoid haing a situation where we can ever get into that case. If we had transactions, then we could just recalculate thr groups, and if two processes happened to do that then thy'd have the same answer, so it doesn't matter. Re dumping for sql errors, see Exception::Class::DBI, + my exceptions thread from sep/oct. We don't want caller() though, but there is a dbi property we can use to get the equiv result.
I think the Postgres patch does this, or could really easily....
Enhancements which don't currently have patches on them which are targetted at 2.18 are being retargetted to 2.20 because we're about to freeze for 2.18. Consideration will be taken for moving items back to 2.18 on a case-by-case basis (but is unlikely for enhancements)
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Yeah, in fact, LockTables() could check if the tables are already locked, if there's a way to do that... That would avoid deadlocks in all situations. It could also throw a warning when it happens, so that we could get an idea of when it's happening. And, of course, it would be easy to remove the inappripriate LOCK TABLES statements, since dkl will know where all of them are. :-)
Depends on: bz-postgres
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004. Anything flagged enhancement that hasn't already landed is being pushed out. If this bug is otherwise ready to land, we'll handle it on a case-by-case basis, please set the blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
In fact, this is done once we start using the bz_lock_tables function, which happens in bug 280503.
As bug 280503 is in, I think this is now fixed. Please, take a look at the solution implemented and if you are happy with it, close this bug.
Bug 280503 covers it.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → WORKSFORME
clearing target of DUPLICATE/WONTFIX/INVALID/WORKSFORME so they'll show up as untriaged if they get reopened.
Target Milestone: Bugzilla 2.20 → ---
You need to log in before you can comment on or make changes to this bug.