Opened 3 years ago

Closed 3 years ago

## #10992 closed defect (fixed)

Reported by: Owned by: jun66j5 hasienda normal WikiCalendarMacro normal i18n threading

### Description

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

### comment:1 follow-up: ↓ 4 Changed 3 years 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)


### comment:2 Changed 3 years 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 3 years ago by hasienda

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: ↓ 5 Changed 3 years ago by rjollos

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 3 years ago by 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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.