Details

Description

It looks like the way apr_xlate is used in APRCharsetEncoder is not thread safe.

It is used when wchar_t is disabled and all LOG4CXX_LOCALE_ENCODING_* defines are set to 0. The bug disappears when wchar_t is enabled (WcstombsCharsetEncoder is used) or when LOG4CXX_LOCALE_ENCODING_UTF8 is set to 1 (TrivialCharsetEncoder is used).

I did not investigate further since no wchar_t and LOG4CXX_LOCALE_ENCODING_UTF8 set to 1 fit my needs.

Activity

I have not been able to reproduce the problem on my available development environments, but there are multiple reports that would suggest that either iconv, apr_iconv or the usage of those in APRCharsetEncoder may not be thread-safe in at least some environments. The links are vague about the error that the Subversion team made in 2002 that seemed to be replicated here. It seems that apr_xlate is thread-safe, but then again it isn't since it affects the memory pool that it was allocated on and that access needs to be synchronized. APRCharsetDecoder seemed to already have been changed to use synchronization blocks around the calls to apr_xlate, so I just modified APRCharsetEncoder to do the same.

I wrote a test that attempted to detect data corruption in the conversion of logchar string to ISO-8859-1. However, the test passed on all environments that I had access to. I would appreciate anyone who had seen data corruption that is attributed to APRCharsetEncoder to attempt to run the test with the old APRCharsetEncoder (prior to 581489) and see if the test fails.

rev 581488: added thread1 test to charsetencodertestcase (but would fail since I left in ISO-8859-1 where it should have been UTF-8)
rev 581489: updated implementation to synchronize access to apr_xlate
rev 581494: modified test to pass when encoding is ISO-8859-1

Curt Arnold
added a comment - 03/Oct/07 06:19 I have not been able to reproduce the problem on my available development environments, but there are multiple reports that would suggest that either iconv, apr_iconv or the usage of those in APRCharsetEncoder may not be thread-safe in at least some environments. The links are vague about the error that the Subversion team made in 2002 that seemed to be replicated here. It seems that apr_xlate is thread-safe, but then again it isn't since it affects the memory pool that it was allocated on and that access needs to be synchronized. APRCharsetDecoder seemed to already have been changed to use synchronization blocks around the calls to apr_xlate, so I just modified APRCharsetEncoder to do the same.
I wrote a test that attempted to detect data corruption in the conversion of logchar string to ISO-8859-1. However, the test passed on all environments that I had access to. I would appreciate anyone who had seen data corruption that is attributed to APRCharsetEncoder to attempt to run the test with the old APRCharsetEncoder (prior to 581489) and see if the test fails.
rev 581488: added thread1 test to charsetencodertestcase (but would fail since I left in ISO-8859-1 where it should have been UTF-8)
rev 581489: updated implementation to synchronize access to apr_xlate
rev 581494: modified test to pass when encoding is ISO-8859-1

configure:[echo] Configuring with has.wchar_t=1[echo] Configuring with has.wcout=1[echo] Configuring with logchar_type=utf-8

build:[cc] Starting dependency analysis for 146 files.[cc] 131 files are up to date.[cc] 15 files to be recompiled from dependency analysis.[cc] 15 total files to be compiled.[cc] Starting link[cc] ar: creating liblog4cxx.a[cc] a - log4cxx_obj/socket.o[cc] a - log4cxx_obj/transcoder.o[cc] a - log4cxx_obj/patternconverter.o[cc] a - log4cxx_obj/optionconverter.o[cc] a - log4cxx_obj/objectptr.o[cc] a - log4cxx_obj/basicconfigurator.o[cc] a - log4cxx_obj/integerpatternconverter.o[cc] a - log4cxx_obj/loggingeventpatternconverter.o[cc] a - log4cxx_obj/filelocationpatternconverter.o[cc] a - log4cxx_obj/fileinputstream.o[cc] a - log4cxx_obj/configurator.o[cc] a - log4cxx_obj/sockethubappender.o[cc] a - log4cxx_obj/bytebuffer.o[cc] a - log4cxx_obj/cyclicbuffer.o[cc] a - log4cxx_obj/triggeringpolicy.o[cc] a - log4cxx_obj/fulllocationpatternconverter.o[cc] a - log4cxx_obj/datelayout.o[cc] a - log4cxx_obj/formattinginfo.o[cc] a - log4cxx_obj/nameabbreviator.o[cc] a - log4cxx_obj/logmanager.o[cc] a - log4cxx_obj/datepatternconverter.o[cc] a - log4cxx_obj/ttcclayout.o[cc] a - log4cxx_obj/writer.o[cc] a - log4cxx_obj/propertyconfigurator.o[cc] a - log4cxx_obj/charsetdecoder.o[cc] a - log4cxx_obj/htmllayout.o[cc] a - log4cxx_obj/dateformat.o[cc] a - log4cxx_obj/loggingevent.o[cc] a - log4cxx_obj/datagramsocket.o[cc] a - log4cxx_obj/fileappender.o[cc] a - log4cxx_obj/transform.o[cc] a - log4cxx_obj/action.o[cc] a - log4cxx_obj/xmlsocketappender.o[cc] a - log4cxx_obj/cacheddateformat.o[cc] a - log4cxx_obj/socketoutputstream.o[cc] a - log4cxx_obj/strftimedateformat.o[cc] a - log4cxx_obj/integer.o[cc] a - log4cxx_obj/odbcappender.o[cc] a - log4cxx_obj/rollingpolicy.o[cc] a - log4cxx_obj/simplelayout.o[cc] a - log4cxx_obj/namepatternconverter.o[cc] a - log4cxx_obj/rolloverdescription.o[cc] a - log4cxx_obj/level.o[cc] a - log4cxx_obj/socketimpl.o[cc] a - log4cxx_obj/systemoutwriter.o[cc] a - log4cxx_obj/writerappender.o[cc] a - log4cxx_obj/loader.o[cc] a - log4cxx_obj/filerenameaction.o[cc] a - log4cxx_obj/simpledateformat.o[cc] a - log4cxx_obj/throwableinformationpatternconverter.o[cc] a - log4cxx_obj/condition.o[cc] a - log4cxx_obj/loggerpatternconverter.o[cc] a - log4cxx_obj/objectimpl.o[cc] a - log4cxx_obj/telnetappender.o[cc] a - log4cxx_obj/filewatchdog.o[cc] a - log4cxx_obj/propertyresourcebundle.o[cc] a - log4cxx_obj/linelocationpatternconverter.o[cc] a - log4cxx_obj/system.o[cc] a - log4cxx_obj/systemerrwriter.o[cc] a - log4cxx_obj/inetaddress.o[cc] a - log4cxx_obj/locationinfo.o[cc] a - log4cxx_obj/onlyonceerrorhandler.o[cc] a - log4cxx_obj/stringmatchfilter.o[cc] a - log4cxx_obj/propertysetter.o[cc] a - log4cxx_obj/mdc.o[cc] a - log4cxx_obj/rootlogger.o[cc] a - log4cxx_obj/filedatepatternconverter.o[cc] a - log4cxx_obj/nteventlogappender.o[cc] a - log4cxx_obj/stringhelper.o[cc] a - log4cxx_obj/dailyrollingfileappender.o[cc] a - log4cxx_obj/bytearrayinputstream.o[cc] a - log4cxx_obj/obsoleterollingfileappender.o[cc] a - log4cxx_obj/patternlayout.o[cc] a - log4cxx_obj/file.o[cc] a - log4cxx_obj/timebasedrollingpolicy.o[cc] a - log4cxx_obj/charsetencoder.o[cc] a - log4cxx_obj/levelpatternconverter.o[cc] a - log4cxx_obj/thread.o[cc] a - log4cxx_obj/serversocket.o[cc] a - log4cxx_obj/locale.o[cc] a - log4cxx_obj/rollingpolicybase.o[cc] a - log4cxx_obj/lineseparatorpatternconverter.o[cc] a - log4cxx_obj/datagrampacket.o[cc] a - log4cxx_obj/fileoutputstream.o[cc] a - log4cxx_obj/propertiespatternconverter.o[cc] a - log4cxx_obj/exception.o[cc] a - log4cxx_obj/xmllayout.o[cc] a - log4cxx_obj/syslogwriter.o[cc] a - log4cxx_obj/relativetimedateformat.o[cc] a - log4cxx_obj/threadspecificdata.o[cc] a - log4cxx_obj/levelmatchfilter.o[cc] a - log4cxx_obj/date.o[cc] a - log4cxx_obj/patternparser.o[cc] a - log4cxx_obj/consoleappender.o[cc] a - log4cxx_obj/socketappenderskeleton.o[cc] a - log4cxx_obj/messagebuffer.o[cc] a - log4cxx_obj/aprinitializer.o[cc] a - log4cxx_obj/layout.o[cc] a - log4cxx_obj/levelrangefilter.o[cc] a - log4cxx_obj/messagepatternconverter.o[cc] a - log4cxx_obj/fallbackerrorhandler.o[cc] a - log4cxx_obj/domconfigurator.o[cc] a - log4cxx_obj/unicodehelper.o[cc] a - log4cxx_obj/threadlocal.o[cc] a - log4cxx_obj/mutex.o[cc] a - log4cxx_obj/syslogappender.o[cc] a - log4cxx_obj/classnamepatternconverter.o[cc] a - log4cxx_obj/rollingfileappender.o[cc] a - log4cxx_obj/socketnode.o[cc] a - log4cxx_obj/resourcebundle.o[cc] a - log4cxx_obj/defaultloggerfactory.o[cc] a - log4cxx_obj/synchronized.o[cc] a - log4cxx_obj/timezone.o[cc] a - log4cxx_obj/pool.o[cc] a - log4cxx_obj/reader.o[cc] a - log4cxx_obj/smtpappender.o[cc] a - log4cxx_obj/filterbasedtriggeringpolicy.o[cc] a - log4cxx_obj/fixedwindowrollingpolicy.o[cc] a - log4cxx_obj/literalpatternconverter.o[cc] a - log4cxx_obj/appenderskeleton.o[cc] a - log4cxx_obj/relativetimepatternconverter.o[cc] a - log4cxx_obj/inputstreamreader.o[cc] a - log4cxx_obj/stringtokenizer.o[cc] a - log4cxx_obj/threadpatternconverter.o[cc] a - log4cxx_obj/properties.o[cc] a - log4cxx_obj/sizebasedtriggeringpolicy.o[cc] a - log4cxx_obj/ndcpatternconverter.o[cc] a - log4cxx_obj/inputstream.o[cc] a - log4cxx_obj/outputstream.o[cc] a - log4cxx_obj/logger.o[cc] a - log4cxx_obj/hierarchy.o[cc] a - log4cxx_obj/ndc.o[cc] a - log4cxx_obj/classregistration.o[cc] a - log4cxx_obj/loglog.o[cc] a - log4cxx_obj/socketinputstream.o[cc] a - log4cxx_obj/class.o[cc] a - log4cxx_obj/asyncappender.o[cc] a - log4cxx_obj/methodlocationpatternconverter.o[cc] a - log4cxx_obj/socketappender.o[cc] a - log4cxx_obj/outputdebugstringappender.o[cc] a - log4cxx_obj/appenderattachableimpl.o[cc] a - log4cxx_obj/manualtriggeringpolicy.o[cc] a - log4cxx_obj/bufferedwriter.o[cc] a - log4cxx_obj/outputstreamwriter.o[cc] a - log4cxx_obj/logstream.o[cc] a - log4cxx_obj/defaultconfigurator.o

build-shortsocketserver:[cc] Starting dependency analysis for 1 files.[cc] 1 files are up to date.[cc] 0 files to be recompiled from dependency analysis.[cc] 0 total files to be compiled.[cc] Starting link

build-cppunit:

get-cppunit-src:

untar-cppunit-src:

os-detect:

win-init:

unix-init:

init:

configure-check:

unix-configure:

win-configure:

configure:

build:[cc] Starting dependency analysis for 49 files.[cc] 49 files are up to date.[cc] 0 files to be recompiled from dependency analysis.[cc] 0 total files to be compiled.

build-unittest:[cc] Starting dependency analysis for 86 files.[cc] 74 files are up to date.[cc] 12 files to be recompiled from dependency analysis.[cc] 13 total files to be compiled.[cc] Starting link

build-defaultinit-unittest:[cc] Starting dependency analysis for 5 files.[cc] 5 files are up to date.[cc] 0 files to be recompiled from dependency analysis.[cc] 0 total files to be compiled.[cc] Starting link

I committed the test first so that you could do exactly the experiment that you just did, but my first commit of the test was in an inconsistent state, converting with ISO-8859-1 but checking against UTF-8 results, which would result every iteration failing. To see if the test actually detects the problem, you need to update charsetencodertestcase.cpp to rev 581494 while charsetencoder.cpp is rev 581488 or earlier.

Curt Arnold
added a comment - 03/Oct/07 17:03 I committed the test first so that you could do exactly the experiment that you just did, but my first commit of the test was in an inconsistent state, converting with ISO-8859-1 but checking against UTF-8 results, which would result every iteration failing. To see if the test actually detects the problem, you need to update charsetencodertestcase.cpp to rev 581494 while charsetencoder.cpp is rev 581488 or earlier.