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.
_______________________________________________
fpc-devel maillist  -  fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel

Reply via email to