RE: [xml] Windows threading issues



Hi Jesse,

My questions (for Stéphane, Igor, and anyone else with an
informed point of
view):

1) Is LIBXML_STATIC_FOR_DLL a suitable name?

No better idea yet...

2) Should the critical section always get set up and torn down in
xmlInitThreads() and xmlCleanupThreads()?  As things stand,
it's done in
DllMain() if libxml is a DLL.  I like the idea of doing it in
one place, but
this introduces the possibility that the CS won't be
initialized when it's
needed if xmlInitParser() isn't called.

2a) If DllMain() continues to set up and tear down the CS,
should the code
be factored out into separate function, since it's identical?

3) Is it sensible and permissible to do the CS setup and tear down in
xmlInitThreads() and xmlCleanupThreads()?  They currently do
nothing, so I'm
not sure I'm using them appropriately.

xmlInitThreads is called from xmlInitParser, and
xmlInitParser *must* be called by a "main" thread before
any other call to libxml, so it should be okay to leave
the initialization in xmlInitParser only.
I did that in the attached patch.

4) Any coding style fax pas?

Daniel and Igor are better judges for this.
There were a few bugs in the linked list manipulations
coming from Eugene's changes.
Fixed in the attached patch.

I also changed the #ifdefs in xmlCleanupThreads
to free the linked list in pure DLL mode too
(in case libxml is shutdown before threads terminate).
Does that makes sense to you?

Finally, I removed the DLL_PROCESS_DETACH code,
because xmlCleanupThreads must have been called before
unloading the dll anyway.
Does that makes sense to you?

Finally, isn't there still a problem for your
original scenario? How will you do the
DLL_THREAD_DETACH dance in LIBXML_STATIC_FOR_DLL mode?
You defer freeing of TLS until xmlCleanupThreads,
but that's a bit late if your process creates and
destroys many threads: TLS memory will accumulate
until libxml shutdown.

Please do some tests and give me feedback on the
attached patch (it applies on top of yours).

Thanks.

-sbi

Attachment: threads.c.2.diff
Description: Binary data



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]