COW thread safety

[zenoyang]

COW thread safety

[lemire]

We are getting there.

On the plus side, when we release, we will have Visual Studio support.

Hang on !!!

[zenoyang]

The preliminary solution is to add pthread_mutex_t mutex variable in shared_container_s, and when the shared container is copied or free, the mutex variable is used for mutual exclusion. Avoid multiple threads modifying counter at the same time. After our POC verification, there is no problem with this solution. I hope you can refer to this plan and give suggestions.

[lemire]

Please review #473

[zenoyang]

Please review #473

Perfect. And I tested it in doris, and I didn't reproduce the assertion failure problem. thanks. @lemire

[lemire]

I have added tests. If it passes the sanitizer tests in CI, I will release.

[SalvatorePreviti]

Question - when #473 is landed, this means all COW operations are thread safe? If so would make sense to enable COW always and remove the flag?

[lemire]

The PR fixing this has been released.

[lemire]

If so would make sense to enable COW always and remove the flag?

That would be a major change that could affect lots of users. I think that the furthest we should go is have a global parameter that can change the default, but I think that COW should remain "opt-in"... unless/until there is widespread consensus that it is best. Even so, it should be done with lots of warning.

It is better not to break existing code.