# HG changeset patch # User Paul Boddie # Date 1442081798 -7200 # Node ID 5be5353060e15d0338dc567a7823a2bf8567acd1 # Parent f3d6831fcc21ae1cecb7e2a734874846046763e8 Changed the locking strategy for handlers to operate exclusively on user stores. This should eliminate the possibility of out-of-sequence object updates and inconsistent free/busy and object details. diff -r f3d6831fcc21 -r 5be5353060e1 imip_store.py --- a/imip_store.py Sat Sep 12 19:58:59 2015 +0200 +++ b/imip_store.py Sat Sep 12 20:16:38 2015 +0200 @@ -554,16 +554,6 @@ return map(lambda t: FreeBusyPeriod(*t), (get_table or self._get_table_atomic)(user, filename, [(4, None)])) - def get_freebusy_for_update(self, user, name=None): - - """ - Get free/busy details for the given 'user', locking the table. Dependent - code must release this lock regardless of it completing successfully. - """ - - self.acquire_lock(user) - return self.get_freebusy(user, name, self._get_table) - def get_freebusy_for_other(self, user, other, get_table=None): "For the given 'user', get free/busy details for the 'other' user." @@ -575,17 +565,6 @@ return map(lambda t: FreeBusyPeriod(*t), (get_table or self._get_table_atomic)(user, filename, [(4, None)])) - def get_freebusy_for_other_for_update(self, user, other): - - """ - For the given 'user', get free/busy details for the 'other' user, - locking the table. Dependent code must release this lock regardless of - it completing successfully. - """ - - self.acquire_lock(user) - return self.get_freebusy_for_other(user, other, self._get_table) - def set_freebusy(self, user, freebusy, name=None, set_table=None): "For the given 'user', set 'freebusy' details." @@ -598,12 +577,6 @@ map(lambda fb: fb.as_tuple(strings_only=True), freebusy)) return True - def set_freebusy_in_update(self, user, freebusy, name=None): - - "For the given 'user', set 'freebusy' details during a compound update." - - return self.set_freebusy(user, freebusy, name, self._set_table) - def set_freebusy_for_other(self, user, freebusy, other, set_table=None): "For the given 'user', set 'freebusy' details for the 'other' user." @@ -616,19 +589,6 @@ map(lambda fb: fb.as_tuple(strings_only=True), freebusy)) return True - def set_freebusy_for_other_in_update(self, user, freebusy, other): - - """ - For the given 'user', set 'freebusy' details for the 'other' user during - a compound update. - """ - - return self.set_freebusy_for_other(user, freebusy, other, self._set_table) - - # Release methods. - - release_freebusy = release_lock - # Tentative free/busy periods related to countering. def get_freebusy_offers(self, user): @@ -641,8 +601,9 @@ # Expire old offers and save the collection if modified. - l = self.get_freebusy_for_update(user, "freebusy-offers") + self.acquire_lock(user) try: + l = self.get_freebusy(user, "freebusy-offers") for fb in l: if fb.expires and get_datetime(fb.expires) <= now: expired.append(fb) @@ -650,35 +611,18 @@ offers.append(fb) if expired: - self.set_freebusy_offers_in_update(user, offers) - + self.set_freebusy_offers(user, offers) finally: - self.release_freebusy(user) + self.release_lock(user) return offers - def get_freebusy_offers_for_update(self, user): - - """ - Get free/busy offers for the given 'user', locking the table. Dependent - code must release this lock regardless of it completing successfully. - """ - - self.acquire_lock(user) - return self.get_freebusy_offers(user) - def set_freebusy_offers(self, user, freebusy): "For the given 'user', set 'freebusy' offers." return self.set_freebusy(user, freebusy, "freebusy-offers") - def set_freebusy_offers_in_update(self, user, freebusy): - - "For the given 'user', set 'freebusy' offers during a compound update." - - return self.set_freebusy_in_update(user, freebusy, "freebusy-offers") - # Object status details access. def _get_requests(self, user, queue): diff -r f3d6831fcc21 -r 5be5353060e1 imiptools/client.py --- a/imiptools/client.py Sat Sep 12 19:58:59 2015 +0200 +++ b/imiptools/client.py Sat Sep 12 20:16:38 2015 +0200 @@ -59,6 +59,16 @@ self.preferences_dir = preferences_dir self.preferences = None + # Store-related methods. + + def acquire_lock(self): + self.store.acquire_lock(self.user) + + def release_lock(self): + self.store.release_lock(self.user) + + # Preferences-related methods. + def get_preferences(self): if not self.preferences and self.user: self.preferences = Preferences(self.user, self.preferences_dir) @@ -673,17 +683,18 @@ if user == self.user: return - freebusy = self.store.get_freebusy_for_other_for_update(self.user, user) + self.acquire_lock() try: + freebusy = self.store.get_freebusy_for_other(self.user, user) fn(freebusy, user, for_organiser, True) # Tidy up any obsolete recurrences. self.remove_freebusy_for_recurrences(freebusy, self.store.get_recurrences(self.user, self.uid)) - self.store.set_freebusy_for_other_in_update(self.user, freebusy, user) + self.store.set_freebusy_for_other(self.user, freebusy, user) finally: - self.store.release_freebusy(self.user) + self.release_lock() def update_freebusy_from_organiser(self, organiser): diff -r f3d6831fcc21 -r 5be5353060e1 imiptools/content.py --- a/imiptools/content.py Sat Sep 12 19:58:59 2015 +0200 +++ b/imiptools/content.py Sat Sep 12 20:16:38 2015 +0200 @@ -72,7 +72,14 @@ handler.set_identity(method) if handler.is_usable(method): - methods[method](handler)() + + # Perform the method in a critical section. + + handler.acquire_lock() + try: + methods[method](handler)() + finally: + handler.release_lock() # Handler registry. diff -r f3d6831fcc21 -r 5be5353060e1 imiptools/handlers/common.py --- a/imiptools/handlers/common.py Sat Sep 12 19:58:59 2015 +0200 +++ b/imiptools/handlers/common.py Sat Sep 12 20:16:38 2015 +0200 @@ -86,29 +86,26 @@ organiser of an event if 'for_organiser' is set to a true value. """ - freebusy = self.store.get_freebusy_for_update(self.user) - try: - # Obtain the attendance attributes for this user, if available. + freebusy = self.store.get_freebusy(self.user) - self.update_freebusy_for_participant(freebusy, self.user, for_organiser) + # Obtain the attendance attributes for this user, if available. - # Remove original recurrence details replaced by additional - # recurrences, as well as obsolete additional recurrences. + self.update_freebusy_for_participant(freebusy, self.user, for_organiser) - self.remove_freebusy_for_recurrences(freebusy, self.store.get_recurrences(self.user, self.uid)) - self.store.set_freebusy_in_update(self.user, freebusy) + # Remove original recurrence details replaced by additional + # recurrences, as well as obsolete additional recurrences. - if self.publisher and self.is_sharing(): - self.publisher.set_freebusy(self.user, freebusy) + self.remove_freebusy_for_recurrences(freebusy, self.store.get_recurrences(self.user, self.uid)) + self.store.set_freebusy(self.user, freebusy) - # Update free/busy provider information if the event may recur - # indefinitely. + if self.publisher and self.is_sharing(): + self.publisher.set_freebusy(self.user, freebusy) - if self.possibly_recurring_indefinitely(): - self.store.append_freebusy_provider(self.user, self.obj) + # Update free/busy provider information if the event may recur + # indefinitely. - finally: - self.store.release_freebusy(self.user) + if self.possibly_recurring_indefinitely(): + self.store.append_freebusy_provider(self.user, self.obj) return True @@ -116,42 +113,36 @@ "Remove free/busy information when handling an object." - freebusy = self.store.get_freebusy_for_update(self.user) - try: - self.remove_from_freebusy(freebusy) - self.remove_freebusy_for_recurrences(freebusy) - self.store.set_freebusy_in_update(self.user, freebusy) + freebusy = self.store.get_freebusy(self.user) - if self.publisher and self.is_sharing(): - self.publisher.set_freebusy(self.user, freebusy) + self.remove_from_freebusy(freebusy) + self.remove_freebusy_for_recurrences(freebusy) + self.store.set_freebusy(self.user, freebusy) - # Update free/busy provider information if the event may recur - # indefinitely. + if self.publisher and self.is_sharing(): + self.publisher.set_freebusy(self.user, freebusy) - if self.possibly_recurring_indefinitely(): - self.store.remove_freebusy_provider(self.user, self.obj) + # Update free/busy provider information if the event may recur + # indefinitely. - finally: - self.store.release_freebusy(self.user) + if self.possibly_recurring_indefinitely(): + self.store.remove_freebusy_provider(self.user, self.obj) def update_event_in_freebusy_offers(self): "Update free/busy offers when handling an object." - freebusy = self.store.get_freebusy_offers_for_update(self.user) - try: - # Obtain the attendance attributes for this user, if available. + freebusy = self.store.get_freebusy_offers(self.user) - self.update_freebusy_for_participant(freebusy, self.user) + # Obtain the attendance attributes for this user, if available. + + self.update_freebusy_for_participant(freebusy, self.user) - # Remove original recurrence details replaced by additional - # recurrences, as well as obsolete additional recurrences. + # Remove original recurrence details replaced by additional + # recurrences, as well as obsolete additional recurrences. - self.remove_freebusy_for_recurrences(freebusy, self.store.get_recurrences(self.user, self.uid)) - self.store.set_freebusy_offers_in_update(self.user, freebusy) - - finally: - self.store.release_freebusy(self.user) + self.remove_freebusy_for_recurrences(freebusy, self.store.get_recurrences(self.user, self.uid)) + self.store.set_freebusy_offers(self.user, freebusy) return True @@ -159,14 +150,11 @@ "Remove free/busy offers when handling an object." - freebusy = self.store.get_freebusy_offers_for_update(self.user) - try: - self.remove_from_freebusy(freebusy) - self.remove_freebusy_for_recurrences(freebusy) - self.store.set_freebusy_offers_in_update(self.user, freebusy) + freebusy = self.store.get_freebusy_offers(self.user) - finally: - self.store.release_freebusy(self.user) + self.remove_from_freebusy(freebusy) + self.remove_freebusy_for_recurrences(freebusy) + self.store.set_freebusy_offers(self.user, freebusy) return True diff -r f3d6831fcc21 -r 5be5353060e1 imiptools/handlers/person.py --- a/imiptools/handlers/person.py Sat Sep 12 19:58:59 2015 +0200 +++ b/imiptools/handlers/person.py Sat Sep 12 20:16:38 2015 +0200 @@ -291,12 +291,9 @@ period = Period(dtstart, dtend, self.get_tzid()) for sender, sender_attr in senders: - stored_freebusy = self.store.get_freebusy_for_other_for_update(self.user, sender) - try: - replace_overlapping(stored_freebusy, period, freebusy) - self.store.set_freebusy_for_other_in_update(self.user, stored_freebusy, sender) - finally: - self.store.release_freebusy(self.user) + stored_freebusy = self.store.get_freebusy_for_other(self.user, sender) + replace_overlapping(stored_freebusy, period, freebusy) + self.store.set_freebusy_for_other(self.user, stored_freebusy, sender) class Freebusy(PersonFreebusy): diff -r f3d6831fcc21 -r 5be5353060e1 imiptools/handlers/resource.py --- a/imiptools/handlers/resource.py Sat Sep 12 19:58:59 2015 +0200 +++ b/imiptools/handlers/resource.py Sat Sep 12 20:16:38 2015 +0200 @@ -103,58 +103,54 @@ periods = self.obj.get_periods(tzid, self.get_window_end()) - freebusy = self.store.get_freebusy_for_update(self.user) - try: - offers = self.store.get_freebusy_offers(self.user) + freebusy = self.store.get_freebusy(self.user) + offers = self.store.get_freebusy_offers(self.user) + + # Check the periods against any scheduled events and against + # any outstanding offers. - # Check the periods against any scheduled events and against - # any outstanding offers. + scheduled = self.can_schedule(freebusy, periods) + scheduled = scheduled and self.can_schedule(offers, periods) - scheduled = self.can_schedule(freebusy, periods) - scheduled = scheduled and self.can_schedule(offers, periods) + # Where the corrected object can be scheduled, issue a counter + # request. - # Where the corrected object can be scheduled, issue a counter - # request. + if scheduled and corrected: + method = "COUNTER" - if scheduled and corrected: - method = "COUNTER" + # Find the next available slot if the event cannot be scheduled. - # Find the next available slot if the event cannot be scheduled. - - #elif not scheduled and len(periods) == 1: + #elif not scheduled and len(periods) == 1: - # # Find a free period, update the object with the details. + # # Find a free period, update the object with the details. - # duration = periods[0].get_duration() - # free = invert_freebusy(freebusy) + # duration = periods[0].get_duration() + # free = invert_freebusy(freebusy) - # for found in periods_from(free, periods[0]): - # # NOTE: Correct the found period first. - # if found.get_duration() >= duration - # scheduled = True - # method = "COUNTER" - # # NOTE: Set the period using the original duration. - # break + # for found in periods_from(free, periods[0]): + # # NOTE: Correct the found period first. + # if found.get_duration() >= duration + # scheduled = True + # method = "COUNTER" + # # NOTE: Set the period using the original duration. + # break - # Update the participation of the resource in the object. - - attendee_attr = self.update_participation(self.obj, - scheduled and "ACCEPTED" or "DECLINED") + # Update the participation of the resource in the object. - # Update free/busy information. + attendee_attr = self.update_participation(self.obj, + scheduled and "ACCEPTED" or "DECLINED") - if method == "REPLY": - self.update_event_in_freebusy(for_organiser=False) - self.remove_event_from_freebusy_offers() + # Update free/busy information. - # For countered proposals, record the offer in the resource's - # free/busy collection. + if method == "REPLY": + self.update_event_in_freebusy(for_organiser=False) + self.remove_event_from_freebusy_offers() - elif method == "COUNTER": - self.update_event_in_freebusy_offers() + # For countered proposals, record the offer in the resource's + # free/busy collection. - finally: - self.store.release_freebusy(self.user) + elif method == "COUNTER": + self.update_event_in_freebusy_offers() # Set the complete event or an additional occurrence.