sashafrey / topicmod

This project had been moved to https://github.com/bigartm/bigartm
Other
0 stars 0 forks source link

Improve thread safety in c_interface #92

Closed sashafrey closed 10 years ago

sashafrey commented 10 years ago

At the moment there are several thread-safety issues in c_interface.

  1. Shared global variable std::string message (shared across GetRequestLength, CopyRequestResult, DisposeRequest, RequetThetaMatrix, RequestTopicModel, RequestScore).

Current behavior of this functions is undefined if the user issues several request from concurrent threads. I suggest the following fix:

Interleaving requests are not valid, e.g. RequestXXX -> RequestYYY -> CopyRequestResult(XXX) -> CopyRequestResult(YYY) is not supported. Indeed, there is no reason for user to do this!

  1. ArtmGetLastErrorMessage

Again, use boost::thread_local_storage to keep the last error message.

JeanPaulShapo commented 10 years ago

@sashafrey Could you explain, why std::string error_message is defined as static, while std::string message is not?

sashafrey commented 10 years ago

This is a mistake. Both variables should be defined as static to prevent their usage outside of the c_interface.cc. The following link explains the difference between "global" and "static global" variables. http://bytes.com/topic/c/answers/860211-global-variable-static-global-variable

sashafrey commented 10 years ago

Fixed in https://github.com/sashafrey/topicmod/commit/278550c2c00e259526d48644e668eccfcd6e9ccb