Module subroutines locking tables cannot be called as part of a wider lock. The (recently gotten rid of) derive_groups hack sporting a tables_already_locked parameter is not a good example imho. Callsites should take all responsibility for locking. For bonus points, we could have a test making sure we don't call bz_lock_tables outside of .cgi files (with the exception of Bugzilla/DB.pm and Bugzilla/DB/*).
I'm a little iffy on this idea... mostly because my *real* goal is just to entirely eliminate table locking, as much as possible, and replace it with transactions.
We should start aiming a little lower, don't you think? If we start moving locking outside of modules, we'll end up with LOCK TABLE calls close to what will turn into START TRANSACTION, and UNLOCK TABLES calls will be near to where COMMIT (or ROLLBACK) will be called. So the changes are along the way.
Morphing, now that we have transactions.
And actually, there's no reason to prevent modules from starting or ending transactions, since we coded our transaction code in such a way that this doesn't matter.