[fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.

[Vinzent Höfler]

Inoussa OUEDRAOGO wrote:

> TAVLTree in avl_tree.pp is not thread safe due to the node
> allocation and de-allocation done through the global
> declared "NodeMemManager" variable. TAVLTreeNodeMemManager
> implementation is cleary not thread safe, which btw IMHO
> is a good thing ( for performance reason).
> 
> Proposition :
> 
> (a) TAVLTree should allow, at construct time, to
>     specify a Node memory manager which will be kept and used.
>     If not specified the global one will be used.
> 
> (b) "NodeMemManager" should be declared as "ThreadVar".

Huh? What's the point of having a global (read: constant) default 
declared as threadvar? So, (b) is unnecessary (at least in your patch).

Another solution would be to declare it as ThreadVar and overwriting it 
upon construction.

But wait... this makes me think if we can't do it automatically: If we 
have a new thread, we need a new instance in most cases anyway (except 
when there's actually only one thread handling XML-data, which I 
consider rather unlikely).

> This changes does not break the API while making the
> implementation thread safe.

Depending solely on the discipline of the caller. And consider the 
XML-case, it wouldn't even be possible without changing this API, too. 
And most likely a lot of other APIs on the way to tackle it down.

> By the way note that the XML DOM's implementation ( TDOMNode_WithChildren )
> uses TAVLTree, so every code that uses the fcl-xml package mainly
> the DOM unit, is not thread safe. If this proposition is accepted
> it will be nice to have it introduced in the soon to be release
> fpc 2.2.2, in the case that is still possible, mainly for server
> programming.

As I just pointed out, it wouldn't make handling XML data threadsafe. 
There's no way to specify upon creation of the TXMLDocument that this 
will be used in another thread, needing a new NodeManager instance.


Vinzent.