apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From SteveKing <stevek...@gmx.ch>
Subject Re: apr-iconv 1.0
Date Wed, 30 Mar 2005 15:49:22 GMT
Branko ─îibej wrote:
> William A. Rowe, Jr. wrote:
> 
>> At 12:36 AM 3/30/2005, Curt Arnold wrote:
>>  
>>
>>> However, it would be good if we could get some comment from a 
>>> Subversion developer.
>>>   
>>
>> Agreed, and Branko's watching this thread.
>>  
>>
> Yup.
> 
> It might also be a good thing to ping Steve King of TortoiseSVN fame; 
> TSVN also ships a hacked version of apr-iconv, I believe the hack is to 
> ignore APR_ICONV_PATH. The issue IIRC is collision between memory 
> allocators in different runtime librararies on Windows; but anyway, 
> Steve should be able to explain.

The reasons why TSVN ships a patched version of apr-iconv:

- the *.so files are actually dll's which require a specific C-runtime. 
This isn't a big problem if APR_ICONV_PATH points to a iconv compiled 
with VC6. But if it points to an iconv compiled with >VC7, then programs 
not using the new c-runtime (> version 7) won't run anymore. A 
'solution' to that problem would be to install the runtime in e.g. the 
system dir on windows, but even MS tells to _not_ do that.

- every program which installs its own iconv lib might change the 
APR_ICONV_PATH variable (believe me: most programs _don't_ check if it's 
already set) and overwrite this with maybe an older version of the lib!

- looking up the env variable isn't very fast, i.e. it takes more time 
to load the *.so modules than other methods.

The changes I made to apr-iconv for TSVN:

- add a DllMain() to iconv_module which caches the path to the *.so 
modules (i.e. the path is determined only once when the iconv dll is 
loaded, not for every *.so module) - this is a _very_ big speed 
improvement for Subversion which makes intensive use of *.so modules.

- the path is determined like this now:
   (in the following description, THISPATH refers to the path where the 
libapriconv.dll is located, fetched via GetModuleFileName() API)
   1. path exists THISPATH\iconv
   2. path exists ..\THISPATH\iconv
   3. use APR_ICONV_PATH
   by doing this, it's assured that a program like TSVN always uses the 
iconv lib it has installed itself. Only if it doesn't install the iconv 
lib itself or it can't be found in 1) or 2), then the APR_ICONV_PATH env 
variable is used to find the *.so modules.

You can find the patched iconv_module.c file here:
https://svn.collab.net/repos/tortoisesvn/trunk/ext/apr-iconv_patch/lib/iconv_module.c

Ever since TSVN ships with this patched iconv version, we haven't had 
any more problems with other Subversion clients interfering or missing 
c-runtime dll's or memory leaks due to incompatible c-runtimes 
(according to MS, the version 6 and 7 of the c-runtime are not 
compatible and use different memory allocators which leads to memory 
leaks and even heap corruptions if a program uses the wrong version).

So I'm not sure what you guys have planned to change in apr-iconv, but I 
strongly recommend that if you change something, get rid of the 
APR_ICONV_PATH (on windows at least), or at least use it as the last 
option like TSVN does now.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.tigris.org

Mime
View raw message