Modify

#10992 closed defect (fixed)

WikiCalendarMacro is not thread-safe

Reported by: jun66j5 Owned by: hasienda
Priority: normal Component: WikiCalendarMacro
Severity: normal Keywords: i18n threading
Cc: Trac Release:

Description

WikiCalendarMacro uses internally locale.setlocale() which is not thread-safe. I think that it should use Babel.

Attachments (0)

Change History (12)

comment:1 follow-up: Changed 21 months ago by jun66j5

Also the following has the same issue, not thread-safe. There is only one active instance of WikiCalendarMacros per each Environment.

    # Returns macro content.
    def expand_macro(self, formatter, name, arguments):

        self.ref = formatter
        self.tz_info = formatter.req.tz
        self.thistime = datetime.datetime.now(self.tz_info)

wikicalendarmacro/trunk/wikicalendar/macros.py#L342

comment:2 Changed 21 months ago by hasienda

(In [12847]) WikiCalendarMacro: Use encoding to get proper weekday headers, refs #10992 and #10993.

Follow-up on [11557], although it might be yet another intermediate step as suggested by #10992. Thanks again to Jun Omae for the patch code and related explainations on the issue.

comment:3 Changed 21 months ago by hasienda

  • Keywords i18n threading added

Babel looks promising so far. I'm half through the process of replacing locale with Locale from babel.core, and there it seems to have answers even to recently asked questions like weekend duration:

>>> from babel import Locale
>>> l = Locale('de')
>>> l
<Locale "de">
>>> l.weekend_start
5
>>> l.weekend_end
6

comment:4 in reply to: ↑ 1 ; follow-up: Changed 21 months ago by rjollos

Replying to jun66j5:

Also the following has the same issue, not thread-safe. There is only one active instance of WikiCalendarMacros per each Environment.

I forget the plugin and ticket in which Jun showed me a similar problem a while back, but it was not something I'd given any thought to prior to that, and it was a good lesson to be learned (though I'm sure I'll make the same mistake again at some point).

In this case it looks trivial to make ref and thistime local rather than instance variables, and tz_info could be made a local variable if the signatures of _mknav and _mkdatetime are changed so that tzinfo is passed as a parameter.

comment:5 in reply to: ↑ 4 Changed 21 months ago by rjollos

Replying to rjollos:

I forget the plugin and ticket in which Jun showed me a similar problem a while back, ...

hasienda, I should have said "showed us" ;) It was #10636, and there was an interesting patch ticket:10636:tagsplugin-t10636-r12375.diff.

comment:6 Changed 21 months ago by hasienda

I did remember almost instantly on sight of the word 'thread-safe'. And I tend to disagree, that we neither can't nor won't do better. I'm inclined to think, that we will remember instead, because this happened to us repeatedly, and we will look out for similar patters more careful in the future.

On the topic: I'm about to commit a comprehensive rework of core calendar build code later today for

  1. transition to Babel objects and methods
  2. removing variables at object level

making it not only thread-safe again, but push it to a new level of localization.

comment:7 Changed 21 months ago by hasienda

(In [12951]) WikiCalendarMacro: Switch from Python core modules calendar and locale towards using Babel by default, refs #10992 and #10993.

This is the bare minimum of changes, that don't break code, but turn Babel into an undeclared, strict requirement so far. I'll fix that later. Starting to remove class attribute variables for becoming thread-safe again.

comment:8 Changed 21 months ago by hasienda

(In [12952]) WikiCalendarMacro: Remove cruft and use datetime.datetime objects even more, refs #10992.

Moving from class methods to functions and changing method/function signatures has been done for trimming main class and finally removing class attributes, that have been abused as variables before (there was even self.tickets!).

comment:9 Changed 21 months ago by hasienda

(In [12953]) WikiCalendarMacro: Do not require Babel, making it an option again, refs #10992.

If Babel is not installed, new compatibility code ensures almost the same behavior like before [12951], but now relying on datetime.datetime alone.

comment:10 Changed 21 months ago by hasienda

Note, that the original issue should have been resolved so far, while subsequently implementing week numbers (#8175) reveals remaining issues.

comment:11 Changed 21 months ago by hasienda

Btw, #1145 is related, as it led to [7324] introducing the non-thread-safe way of dealing with different 1st-day-of-week settings for WikiCalendarMacro in the first place.

comment:12 Changed 20 months ago by hasienda

  • Resolution set to fixed
  • Status changed from new to closed

(In [12991]) WikiCalendarMacro: Releasing current, tested macro package as final product, closes #7639, #8175, #9718, #10991, #10992 and #10993.

Special thanks to Jun Omae for pushing development by testing and providing valuable hints in our discussion about utilizing Babel for better localization and for making macro execution thread-safe as well.

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.