hraban / cl-containers

Containers Library for Common Lisp
http://common-lisp.net/project/cl-containers/
Other
64 stars 13 forks source link

Fix concurrency issue in red-black-tree #7

Closed lokedhs closed 9 years ago

lokedhs commented 9 years ago

The old implementation used a global RBT-EMPTY-NODE for all instances of red-black-tree. This caused an error when multiple threads were updating different trees at the same time.

Further discussion here: https://github.com/gwkkwg/cl-containers/issues/6

fare commented 9 years ago

Apart from the debugging declaration, it looks good to me (though I gave a somewhat superficial look assuming you had tested it).

Speaking of tests, can you add your test cases to the test suite?

lokedhs commented 9 years ago

I was thinking about that, but I have to admit that I'm not sure how to do that using the test framework since the bug was triggered when starting multiple threads. Thus the exception wouldn't be picked up by the unit tests.

lokedhs commented 9 years ago

I've removed the debug messages.

gwkkwg commented 9 years ago

Thank you Elias for finding and, even better, fixing this problem. And thank you Faré for merging it in.