Recommended Posts

I would second the advice to remove all global variables. This is actually evil and not a good-practice anyway.

I would suggest you first make Devil object-oriented. This will first remove the need of these global variables, and will consolidate Devil architecture.

Once the project has been better structured, you will be able to identify pieces of code that might need mutex protection.

This might sound like a lot of work (even if not so many classes would be required - from what I know from the source code), but I think this is actually mandatory to make Devil stable, structured and easier to use. Also, once done, making it thread-safe would be much easier than as in its current state.

Share this post

Link to post

Share on other sites

Original post by gjaegyI would second the advice to remove all global variables. This is actually evil and not a good-practice anyway.

I would suggest you first make Devil object-oriented. This will first remove the need of these global variables, and will consolidate Devil architecture.

Once the project has been better structured, you will be able to identify pieces of code that might need mutex protection.

This might sound like a lot of work (even if not so many classes would be required - from what I know from the source code), but I think this is actually mandatory to make Devil stable, structured and easier to use. Also, once done, making it thread-safe would be much easier than as in its current state.

Just my personal opinion though...

I agree. I have been thinking the past few days about rewriting it in C++, though it would still have a C-style interface so that other languages can use it. It will be a lot of work, and it will probably take awhile, since I am really busy with school. Thanks for your thoughts on it.

0

Share this post

Link to post

Share on other sites

There is a lot of misguided advice on concurrent programming, so be wary of the web. The best way to achieve robust multi-threading is to make sure your software architecture is sound. Except for the global vars, everything seems fine. It also seems like these variables should indeed be global, which means having different instances per thread is not what you want. 1) Make the global variables encapsulated with proceedures. 2) On access to these variables, place a lock (mutex, Critical Section, etc.), copy the values out or set the values and then unlock. If you can copy the data over then things are much safer.

Things to avoid: a) avoid locks everywhere. By placing a single lock in the encapsulated method that makes no calls back out to the system (or itself) you avoid the possibility of deadlock. Immutable data structures are a parallel programmers dream.b) Worrying about performance. The performance warnings usually come from systems people who worry about microseconds and view the world as constantly calling locks. For your situation it seems that you rarely will call a lock, so it is a non-issue. Note, for the Win32 critical section, if the lock is not currently held it boils down to a no-op. Hence it is very very fast. Contention for locks will always cause performance problems, so it is best to limit the contention. The above should do that.

For the encapsulation, Object oriented would help (as would the Singleton Design Pattern), but you can get by with a single point of entry (a C function).

Protect your data and you should be okay.

0

Share this post

Link to post

Share on other sites

Here is some C++ code that uses pThreads and MS Critical section. For C, encapsulate a separate _lock variable for each logic chunk of global memory you want to protect. Also hide the lock call and access to the _lock variable behind your encapsulated global variable. Except for the constructors, everything else is just a facade over the existing system support.

FYI, I am a Professor of Computer Science at The Ohio State University and do not usually check the forums. Some students were having some problems with DevIL, so I noticed your plea for help there. Great library so hopefully this is giving back a little.

Share this post

Link to post

Share on other sites

Original post by gjaegyFYI I am using boost::mutex and their implementation is quite good.

I guess you don't want to introduce any dependency to boost in the library, but it might give you another implementation inspiration ;)

Can't believe I missed this before, but you could use boost::mutex without dedicating yourself to a long-term dependency problem, because it's being adopted as the standard in C++09 (as well as boost::thread). And from what I understand a portable sockets implementation for inter-process communication will be in there as well.

The simplest approach to this is to provide context, and move each and every global into it (actually, only non-const/non-read-only variables). The API is then modified by requiring this context in every function call.

The change requires addition of create/destroy functions, and modification of public API with an extra parameter. The modifications could look something like this:

While API breaking change, it has several advantages:- API change can effectively be ignored by those not interested into threading- Old-style API can be provided for compatibility where ILContext is still global- Thread-safety is possible, but left to the user if needed- It does not require any specific thread or OS library- Change is fairly mechanical and simple to perform (move all statics into ILContext)- The change itself requires minimal testing since aside from putting variables into a struct nothing changed

How to use such library in threaded environment?- allocate ILContext inside TLS- let user acquire locks as they choose, with the API they choose, around the operations they need to- via C++ RAII wrapper than encapsulates entire operations- up to user

Quote:

There are a lot of parameters that you can set via ilEnable/ilDisable and ilSetInteger.

The reason why I suggest this approach has to do with ACID problem. Even if each individual ilEnable call is made thread-safe, the sequence of operations itself isn't a transaction, and cannot be made into one without changing the API considerably.

By moving global state into user-allocated object (see any major C API, mysql or such), user gets full control over how and when internal state is modified. That gives sufficient and required control that is needed for thread-safe use.