Issue Details (XML | Word | Printable)

Key: CORE-2642
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Adriano dos Santos Fernandes
Reporter: Kovalenko Dmitry
Votes: 0
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
Firebird Core

ICU initialization for properly work in MT environment

Created: 26/Sep/09 11:52 PM   Updated: 12/Nov/09 05:44 PM
Component/s: Charsets/Collation
Affects Version/s: 2.1.0, 2.1.1, 2.1.2, 2.5 Beta 1, 2.5 Beta 2, 2.1.3
Fix Version/s: 2.5 RC1

Time Tracking:
Not Specified

File Attachments: 1. Text File module_initialization.cpp (1 kB)

Environment: Multithread stress tests

Planning Status: Unspecified


 Description  « Hide
At durring a multithreaded stress tests I got the problems with ICU. Later I found in extern\icu\readme.html the next informations

-
Using ICU in a Multithreaded Environment

Upon the first usage of most ICU APIs, the global mutex will get initialized properly, but you can use the u_init() function from uclean.h to ensure that it is initialized properly. Without calling this function from a single thread, the data caches inside ICU may get initialized more than once from multiple threads, which may cause memory leaks and other problems. There is no harm in calling u_init() in a single threaded application.

-
I added in to icuuc30.dll the DllMain function (see attach) and made two pass of my tests - all work fine without any problems.

(But I use the umtx_init instead u_init, because u_init can return the error at durring ICU compilations).

Also, I added the call of umtx_cleanup for releasing the resources of ICU critical sections. With u_cleanup I got the problem in x64 build.

Could you add this (or similar) code to your ICU "common" project?

 All   Comments   Work Log   Change History   Version Control   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Kovalenko Dmitry added a comment - 26/Sep/09 11:56 PM
DllMain for icuuc30.dll

Adriano dos Santos Fernandes added a comment - 27/Sep/09 01:43 AM
Calling umtx_init(NULL) is related to implementation details not documented. u_init was been marked as DRAFT in 3.0, but apparently was been later marked as stable since 2.6. What compilation problem did you had?

But u_cleanup, IMO, may cause more harm than good. Suppose one application loads fbembed and ICU, then unload fbembed. u_cleanup does not do referencing counting, so it will clean ICU used by the application.

For the DllMain, that is how really they should do it. But I would not like for us to fix all they problems, this one, for example, has a very low surface: fbembed being loaded and unloaded will cause leaks.

I'll verify more recent versions and report a bug for them if appropriate.

Kovalenko Dmitry added a comment - 27/Sep/09 02:21 AM
>Calling umtx_init(NULL) is related to implementation details not documented.

documentation - for external users. "DllMain" - this is "internal" user :)

> u_init was been marked as DRAFT in 3.0, but apparently was been later marked as stable since 2.6. What compilation problem did you had?

u_init returns the status code. When I tried to detect and process fail-status-code (and return the FALSE from DllMain) genrb.exe was fail

---
IBProvider+fbembed can to use ICU library together

So, ICU DLL should independently initialize and deinitialize the own global internal resources (for example, in DllMain code)

This is general rule for all normal DLLs.

----
As I understand (but not sure) u_cleanup produces the next problem:
 - icuin30.dll registers the callback function in icuuc30.dll
 - icuin30.dll unloads from memory
 - icuuc30.dll calls the function by wrong pointer

Then I decide do not use the u_cleanup, and use the umtx_cleanup only

----
Thank you.

Adriano dos Santos Fernandes added a comment - 05/Oct/09 01:26 AM
Fixed only the initialization problem, and not the cleanup.

I'd hate to change ICU sources, and it will not work for Linux distributions that uses system ICU anyway.