Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve thread safety in c_interface #92

Closed
sashafrey opened this issue Aug 17, 2014 · 3 comments
Closed

Improve thread safety in c_interface #92

sashafrey opened this issue Aug 17, 2014 · 3 comments
Assignees
Labels

Comments

@sashafrey
Copy link
Owner

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:

  • keep "std::string message" in boost::thread_local_storage. This allows user to use several concurrent threads to issue requests in parallel.
  • Remove functions GetRequestLengt and DisposeRequest, because they can be avoided. To achieve this, change RequestXXX functions to return the length of the request instead of the "request id". If request failed, return the error code, which is a negative value - and therefore it can be distinguished from successful request (non-negative return value). Document, that each use thread must use the following sequence to retrieve request:
    RequestXXX -> allocate buffer -> CopyRequestResult.

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.

@sashafrey sashafrey added the bug label Aug 17, 2014
@sashafrey sashafrey self-assigned this Aug 18, 2014
@JeanPaulShapo
Copy link
Collaborator

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

@sashafrey
Copy link
Owner Author

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
Copy link
Owner Author

Fixed in 278550c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants