[fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
JeLlyFish.software at gmx.net
Wed Aug 6 22:56:27 CEST 2008
Inoussa OUEDRAOGO wrote:
> 2008/8/6 Mattias Gaertner <nc-gaertnma at netcologne.de>:
>> On Wed, 6 Aug 2008 19:41:27 +0100
>> Providing a local node mem manager does not repair xml dom.
> A "threadvar" declared variable is local to the calling thread.
Yes. But the reference this threadvar is pointing to is _not_.
ThreadVar don't help here unless you provide them with a different node
manager reference in each thread which is kind of senselessly redundant
considering that your patch already can provide a local reference for
each instance created.
And even so, the XML code does not call this new API unless changed, so
your patch actually changes nothing in regard to the XML data.
It only works in cases where I directly instantiate an AVLTree. Or, how
else is me - the programmer - supposed to know if a class uses an
AVLTree or not? And even if I knew that (by digging through the whole
source), I couldn't do anything about it, because the classes don't
provide that interface to me. IOW: I don't have control over it.
[moving NodeManager to interface]
> Good solution, but : if ThreadA create an avl object that is _exclusively_ used
> in ThreadA, why should ThreadA wait in a multithreaded node mem manager?
That's where a ThreadVar becomes useful: Each thread could install its
personal instance of the (unsafe) NodeMemManager via this "global" (yet
thread-local) reference. No further changes to the code needed, but the
programmer must be aware of this problem each time he creates a thread
using any instance of the AVLTree directly or indirectly.
That's surely not a nice solution, but something you can at least work
with. Although it still smells rotten to me.
> That is when comes the constructor that takes a node mem manager.
> That way, ThreadA could provide a local node mem manager that is not
> multithreaded and thus not paying penalty of multithreaded node mem manager.
That's theoretically possible (but then again, this is perfectly
solvable without ThreadVars), but this change in the API must then be
followed up to *any* class that /might/ use an AVLTree instance to be
any effective. I'd say: No way to do that, especially not in the
timeframe you propose. And then you may consider the required changes to
all the existing code, to make that actually thread-safe...
IMO, the least intrusive change would be to define the NodeManager as
ThreadVar and let it "somehow" determine if it's running in a new thread
and then overwrite the thread local variable with a new instance. It
would leave all existing interfaces untouched and even would make
already existing code automatically thread-safe in that regard.
Of course, removing the usage of the AVLTree altogether would achieve
about the same, but would be a slightly more intrusive change.
More information about the fpc-devel