Database version updates drop nomail settings

There's a small bug in 2.1alpha4 in updating a list's database to the DATA_FILE_VERSION, causing "nomail" settings to not be propagated forward:
MailList.CheckVersion() reloads the database, taking care to make sure that the reload doesn't trigger a recursive call to CheckVersion(). But if the list wasn't locked, CheckVersion() then calls Lock(), and Lock() calls Load() again, this time generating a recursive call to CheckVersion(). This recursion is only one deep because now the list is locked, but even that is too much for versions.CanonicalizeUserOptions() since it clears the old-style "nomail" flag after setting the delivery status in the new database.
Here's a patch that fixes this.
-les
*** MailList.py.orig Sun Jan 6 23:04:01 2002 --- MailList.py Sat Feb 9 22:42:36 2002
*** 146,157 **** # # Lock management # ! def Lock(self, timeout=0): self.__lock.lock(timeout) # Must reload our database for consistency. Watch out for lists that # don't exist. try: ! self.Load() except Exception: self.Unlock() raise --- 146,157 ---- # # Lock management # ! def Lock(self, timeout=0, check_version=1): self.__lock.lock(timeout) # Must reload our database for consistency. Watch out for lists that # don't exist. try: ! self.Load(check_version) except Exception: self.Unlock() raise
*** 557,563 **** # We must hold the list lock in order to update the schema waslocked = self.Locked() if not waslocked: ! self.Lock() try: from versions import Update Update(self, stored_state) --- 558,564 ---- # We must hold the list lock in order to update the schema waslocked = self.Locked() if not waslocked: ! self.Lock(check_version=0) try: from versions import Update Update(self, stored_state)

"LN" == Les Niles <les@2pi.org> writes:
LN> There's a small bug in 2.1alpha4 in updating a list's database
LN> to the DATA_FILE_VERSION, causing "nomail" settings to not be
LN> propagated forward:
LN> MailList.CheckVersion() reloads the database, taking care to
LN> make sure that the reload doesn't trigger a recursive call to
LN> CheckVersion(). But if the list wasn't locked, CheckVersion()
LN> then calls Lock(), and Lock() calls Load() again, this time
LN> generating a recursive call to CheckVersion(). This recursion
LN> is only one deep because now the list is locked, but even that
LN> is too much for versions.CanonicalizeUserOptions() since it
LN> clears the old-style "nomail" flag after setting the delivery
LN> status in the new database.
I need more help with this one because I cannot reproduce the problem. I've tried taking a MM2.0.8 list, with some members delivery disabled, and done upgrades to cvs. I've tried loading the list in its locked and unlocked state. In every case I see the disabled flag propagate to the updated list. Of course, the member management page shows the reason for disabled as [?] which is correct.
I have a tentative patch to versions.py that at least stops CanonicalizeUserOptions() from running more than once. But I can't judge the correctness of Les's patch unless I have a reproducible bug. So if you have some explicit steps to trigger the bug, please send them on, otherwise, I can't do much about this.
If it's a real bug, I'm sure it will come up during beta testing, but that might be too late (i.e. it might bite badly for people who are, er, more enthusiastic about upgrading. ;).
-Barry

On Sat, 9 Mar 2002 12:30:58 -0500 barry@zope.com (Barry A. Warsaw) wrote:
It looks like you've already fixed it in CVS. I just confirmed that I could reproduce the original bug after backing out my patch. (Actually I brain-farted and first tried to reproduce it without backing out the patch....) But when I hacked versions.py to initialize delivery_status with add_only_if_missing() as is done in the current CVS version -- in 2.1alpha4 delivery_status was just set to {} -- the problem disappeared. (I left the add_only_if_missing() in my code, since that seems like a much better fix than mucking around with MailList.Lock().)
BTW, the way I produced the problem was just by copying a lists/<listname> directory -- for a list with some disabled-delivery subscribers -- from 2.0beta6 to 2.1alpha4.
Yeah, it tended to cause unrest when folks had set up disabled- delivery subscriptions to be able to post from alternate addresses and all of a sudden started getting multiple copies of every message. ;)
-les

"LN" == Les Niles <les@2pi.org> writes:
LN> BTW, the way I produced the problem was just by copying a
LN> lists/<listname> directory -- for a list with some
LN> disabled-delivery subscribers -- from 2.0beta6 to 2.1alpha4.
Cool. I'm much more confident that the bug is now squashed in cvs. I've done this several times (actually copied a list from 2.0.8 to 2.1cvs) and with different combinations of nomail settings, and all the upgrades seem to work.
>> If it's a real bug, I'm sure it will come up during beta
>> testing, but that might be too late (i.e. it might bite badly
>> for people who are, er, more enthusiastic about upgrading. ;).
LN> Yeah, it tended to cause unrest when folks had set up
LN> disabled- delivery subscriptions to be able to post from
LN> alternate addresses and all of a sudden started getting
LN> multiple copies of every message. ;)
Yeah, that would suck. :)
Thanks, -Barry

"LN" == Les Niles <les@2pi.org> writes:
LN> There's a small bug in 2.1alpha4 in updating a list's database
LN> to the DATA_FILE_VERSION, causing "nomail" settings to not be
LN> propagated forward:
LN> MailList.CheckVersion() reloads the database, taking care to
LN> make sure that the reload doesn't trigger a recursive call to
LN> CheckVersion(). But if the list wasn't locked, CheckVersion()
LN> then calls Lock(), and Lock() calls Load() again, this time
LN> generating a recursive call to CheckVersion(). This recursion
LN> is only one deep because now the list is locked, but even that
LN> is too much for versions.CanonicalizeUserOptions() since it
LN> clears the old-style "nomail" flag after setting the delivery
LN> status in the new database.
I need more help with this one because I cannot reproduce the problem. I've tried taking a MM2.0.8 list, with some members delivery disabled, and done upgrades to cvs. I've tried loading the list in its locked and unlocked state. In every case I see the disabled flag propagate to the updated list. Of course, the member management page shows the reason for disabled as [?] which is correct.
I have a tentative patch to versions.py that at least stops CanonicalizeUserOptions() from running more than once. But I can't judge the correctness of Les's patch unless I have a reproducible bug. So if you have some explicit steps to trigger the bug, please send them on, otherwise, I can't do much about this.
If it's a real bug, I'm sure it will come up during beta testing, but that might be too late (i.e. it might bite badly for people who are, er, more enthusiastic about upgrading. ;).
-Barry

On Sat, 9 Mar 2002 12:30:58 -0500 barry@zope.com (Barry A. Warsaw) wrote:
It looks like you've already fixed it in CVS. I just confirmed that I could reproduce the original bug after backing out my patch. (Actually I brain-farted and first tried to reproduce it without backing out the patch....) But when I hacked versions.py to initialize delivery_status with add_only_if_missing() as is done in the current CVS version -- in 2.1alpha4 delivery_status was just set to {} -- the problem disappeared. (I left the add_only_if_missing() in my code, since that seems like a much better fix than mucking around with MailList.Lock().)
BTW, the way I produced the problem was just by copying a lists/<listname> directory -- for a list with some disabled-delivery subscribers -- from 2.0beta6 to 2.1alpha4.
Yeah, it tended to cause unrest when folks had set up disabled- delivery subscriptions to be able to post from alternate addresses and all of a sudden started getting multiple copies of every message. ;)
-les

"LN" == Les Niles <les@2pi.org> writes:
LN> BTW, the way I produced the problem was just by copying a
LN> lists/<listname> directory -- for a list with some
LN> disabled-delivery subscribers -- from 2.0beta6 to 2.1alpha4.
Cool. I'm much more confident that the bug is now squashed in cvs. I've done this several times (actually copied a list from 2.0.8 to 2.1cvs) and with different combinations of nomail settings, and all the upgrades seem to work.
>> If it's a real bug, I'm sure it will come up during beta
>> testing, but that might be too late (i.e. it might bite badly
>> for people who are, er, more enthusiastic about upgrading. ;).
LN> Yeah, it tended to cause unrest when folks had set up
LN> disabled- delivery subscriptions to be able to post from
LN> alternate addresses and all of a sudden started getting
LN> multiple copies of every message. ;)
Yeah, that would suck. :)
Thanks, -Barry
participants (3)
-
barry@zope.com
-
Dan Mick
-
Les Niles