Thank you for tackling the multiple Init/Term calls issue. The
best format for
submitting your patch is to post it to the mailing list, and a Xerces-C
committer
will commit the changes to CVS acknowledging your contribution.
Khaled
Mark Weaver wrote:
OK, I've implemented my patch to allow multiple Init/Term calls. As far as
the use of static data goes, Jesse provided this list which I've looked
over:> 1) src\com\XMLDOMUtil.cpp: m_pool is a static template member. The code
is
> complex, and I didn't try to analyze it. I'm disinclined to worry about
it,
> since I think it's a COM wrapper around the Xerces library, and it's the
> library that has the problem.
Yes, this is a COM wrapper and it is the COM object that has the problem.
I'm not certain, but I can't see anyway that the pooled objects actually get
destroyed when the object is unloaded.> 2) src\dom\DOMString.hpp: gLiveStringHandleCount could conceivably be a
> problem in an ill-behaved app, because DOMStringTerminate() won't get
called
> unless the count goes to zero. It's not obvious to me what kind of
> misbehavior would cause this, or what an appropriate response might be.
It might be worth someone adding a DOMStringHandle::checkTermination() or
some such behaviour which can report an error from Terminate() (the return
type would need to change).> 3) src\dom\DOMString.hpp: gLiveStringDataCount likewise might not go to
> zero. It exists solely for the benefit of DomMemDebug.
This is as (2)> 4, 5) src\dom\DOMString.hpp: gTotalStringHandleCount and
> gTotalStringDataCount are always incremented, never decremented, and exist
> only for the benefit of DomMemDebug. It's unclear to me whether they
should
> be cleared in Terminate().
Since they are only incremented they are bound to be incorrect so I guess we
are seeing obsolete code.> 6) src\dom\NamedNodeMapImpl.hpp: gLiveNamedNodeMaps - like
> gLiveStringDataCount.
As (2)> 7) src\dom\NamedNodeMapImpl.hpp: gTotalNamedNodeMaps - like
> gTotalStringHandleCount and gTotalStringDataCount.
As (4,5)> 8) src\dom\NodeImpl.hpp: gLiveNodeImpls - like gLiveStringDataCount.
As (2)> 9) src\dom\NodeImpl.hpp: gTotalNodeImpls - like gTotalStringHandleCount
and
> gTotalStringDataCount.
As (4,5)> 10) src\internal\XMLScanner.cpp: gScannerId - incremented but never
> decremented. Might be nice to reset to zero on termination, but harmless
> not to (I think).
I don't think that this is one to worry about :-)> 11) src\internal\XMLScanner.cpp: registered is set true when a scanner
mutex
> is first allocated. The mutex gets deallocated during termination, but
> registered remains true. This is the most serious example of the problem
> you describe.
Fixed (see list below).> 12, 13, 14, 15) src\util\Platforms\HPUX\HPPlatformUtils.cpp,
> src\util\Platforms\IRIX\IRIXPlatformUtils.cpp,
> src\util\Platforms\Linux\LinuxPlatformUtils.cpp,
> src\util\Platforms\UnixWare\UnixWarePlatformUtils.cpp: atomicOpsMutex gets
a
> mutex allocated but never released. Presumably could be cleaned up in
> platformTerm().
Fixed.> 16, 17, 18) src\util\Platforms\OS400\OS400PlatformUtils.cpp,
> src\util\Platforms\Solaris\SolarisPlatformUtils.cpp,
> src\util\Platforms\Tru64\Tru64PlatformUtils.cpp: gAtomicOpMutex is
allocated
> but never released. Presumably could be cleaned up in platformTerm().
Fixed.> 19) src\util\Transcoders\Iconv400\iconv_util.c: _defaultConverter gets
> opened but never closed. This code is, umm, not up to the usual Xerces
> standards, and I have no idea how to fix it.
> 20) src\util\Transcoders\Iconv400\iconv_util.c: gErr appears to get set
> (when it's passed by reference to a function) but never used. It's not
> clear whether its value is of any significance. Whether anything needs to
> be done, or how, is unclear.
>
> 21) src\util\Transcoders\Iconv400\iconv_util.c: DEFAULT_CONVERTER_NAME
gets
> initialized with strcpy() and never modified thereafter; I'm not sure why
> it's not initialized when declared and made const. (There are a fair
number
> of other instances where it appears that data could be const, but isn't.)
This file is _almost_ too bletcherous to contemplate. It's in C, which
presents the main problem, but is OS400 specific so I fixed this by a direct
call from OS400PlatformUtils' platformTerm to a cleanup function. This is
#ifdefd to be called only if you are using that converter. I suspect there
may be other stuff like this lurking around, but I'm not motivated enough to
try fix it (and I can't compile it, so it would be doubly tricky).Right, apart from the platform specific stuff, these are the classes I
caught using static/global data:DOM_DOMImplementation uses gDomimp
IDOM_DOMImplementation uses gDomimp
XMLScanner uses sRegistered, sScannerMutex
XMLException uses sLoader, sMsgMutex
RangeTokenMap uses fInstance
DatatypeValidatorFactory uses fBuiltInRegistry, fRegistryExpanded
GeneralAttributeCheck uses fInstanceNow hopefully I've been thorough enough to catch everything, if anyone else
knows of anything else like this then please let me know.What is the best format to submit patches in/should they go to the list?
Thanks,
Mark
> -----Original Message-----
> From: Jesse Pelton [mailto:[EMAIL PROTECTED]]
> Sent: 21 September 2001 16:22
> To: '[EMAIL PROTECTED]'
> Subject: RE: using differnt instances of the parser causes ACCESS
> Violation
>
>
> The idea of static cleanup objects dynamically added to a list as needed
> makes sense, though I'm agnostic on the question of whether the objects
> themselves should be static. (There aren't that many and they aren't that
> big, so it can't make much difference either way.) There should be no
> construction order issues, because all the static objects (including the
> list object) should be initialized before we start running code
> that lazily
> allocates memory. (FWIW, I now think you're right that construction order
> wasn't an issue with the previous approach of building the list as static
> objects are constructed. I suspect I was incorrectly thinking of
> gXMLCleanupList as an object that needed to be constructed, rather than a
> simple pointer. But you're right, it's cleaner not to add cleanups to the
> list until you know you need them.)
>
> Note, though, that there may be more than 8 instances where cleanups are
> needed. I listed 21 candidates for cleanup in
> http://marc.theaimsgroup.com/?l=xerces-c-dev&m=86952115511532&w=2.
> Some may
> have been added or removed, and some may not actually need to be
> addressed.
> As long as you've looked over my list and reviewed the code for all
> platforms, I'm happy.
>
> Thanks for taking this on. This is one of those issues that generates far
> too much traffic on the list.
>
> -jesse-
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
