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 - [email protected] http://lists.freepascal.org/mailman/listinfo/fpc-devel
