11

I have a situation in C as well as in C++ which can be best solved with something like the Python like decorators: I have few a functions which I'd like to wrap around with something else so that before the function enters some statements are performs and when it leaves some other functionality is executed.

For instance, I have a few functions in a library C file which when called should lock a semaphore and before returning the control to callee, should release the semaphore. without the lock they have following structure:

int f1(int)
{
    ...
    ... 
}

int f2(char*)
{
    ....
}

int f3(blabla)
{
    ....
}

... fn(...)

I'd like to define a global semaphore which should be locked before each of these functions is called and released when the function is returned. I'd like to do it with as much simplicity as possible; something close to this:

#lockprotected
int f1(int)
{
   ... /* nothing changed over here */
}
#endlockprotected

or something like

int f1(int)
{
   ... /* nothing changed over here */
}
#lockprotected f1

What I don't want is:

  1. Change the function names as they are library functions and are being called from many places.
  2. Explicitly place any statement before return calls as most of the functions have many early returns in between. Or for that matter change any internals of the function.

What would be the most elegant way?

sharjeel
  • 5,825
  • 7
  • 34
  • 49
  • 4
    You have to decide C or C++, the approaches will be different, with C++ you can use RAII as mentioned below, and with C, you have to move the implementation to separate functions and the originals simply become wrappers that have lock/unlock around the call to the real implementation - variation of @frunsi's answer below – Nim Jan 12 '11 at 10:13

7 Answers7

17

Use RAII (resource acquisition is initialization) to define the lock on the mutex. This would allow you to forget about point #2, i.e., you don't need to keep track of return statement to release the lock.

class Lock {
public:
    Lock () { // acquire the semaphore }
    ~Lock () { // release the semaphore }
}

Next create objects of this class at the start of your functions, i.e.,

int f1 (int) {
    Lock l;
    // you can now forget about release of this lock
    // as ~Lock() will take care of it
}

A side advantage of this is that even in the case of exceptions being thrown from f1(), you still don't need to worry about releasing the lock. All stack objects are destroyed before a function exits.

Jaywalker
  • 3,079
  • 3
  • 28
  • 44
  • 2
    +1 RAII is the C++ way of doing it. The f1 function must also be wrapped. Which can be done with a simple template function (will obfuscate the code a little bit) or just by creating simple wrappers as frunsi explained. – daramarak Jan 12 '11 at 10:07
  • 3
    one obvious issue, is that you may then be protecting too many functions... and mutexes are not reentrant in general. – Matthieu M. Jan 12 '11 at 10:38
  • 1
    Please heed [Matthieu M.](http://stackoverflow.com/questions/4667293/how-to-implement-decorators-in-c-and-c/4667844#4667844)'s advice and use [Reentrant Mutex](http://stackoverflow.com/questions/1312259/what-is-the-re-entrant-lock-and-concept-in-general). It allows a single thread to grab the mutex any number of times, while blocking other threads that try to call the library function. Otherwise, there is a risk of self-deadlocking from just one thread. – rwong Mar 04 '11 at 06:48
  • @daramarak, why do we need to wrap the f1 function? I don't quite understand. Can you please clarify? – batbrat Feb 10 '14 at 10:48
  • 1
    @batbrat To prevent changing the f1 function. It is better to seperate the concern of locking to a wrapping function. Also, the OP specifies that "nothing changed over here" which might imply that the function in one way or another should not be touched. – daramarak Feb 10 '14 at 12:51
12

if you really want a C solution, you could go with macros like:

#define LOCK   lock( &yourglobalsemaphore )
#define UNLOCK unlock( &yourglobalsemaphore )

#define LOCKED_FUNCTION_ARG1(TRet, FuncName, TArg1, Arg1Name ) \
TRet FuncName( TArg1 Arg1Name ) { \
 LOCK; \
 TRet ret = FuncName##_Locked( Arg1Name ); \
 UNLOCK; \
 return ret \
} \
TRet FuncName##_Locked(TArg1 Arg1Name )

#define LOCKED_FUNCTION_ARG2(TRet FuncName, TArg1, Arg1Name, TArg2, Arg2Name) \
//...etc

but you will need 1 macro for every count of arguments (and the function should have a return type).

example usage:

LOCKED_FUNCTION_ARG1(int, f1, int, myintarg)
{
   //unchanged code here
}
smerlin
  • 6,446
  • 3
  • 35
  • 58
  • 1
    `FuncName_Locked` will appear exactly as you typed it because it is not a macro parameter. You need to concatenate FuncName and _Locked with the `##` macro operator. – JeremyP Jan 12 '11 at 11:44
  • If using C++, combines it with boost::preprocessor would be a promising solution. – Bear Dec 27 '13 at 18:10
3

Define the wrapper like this:

class SemaphoreWrapper
{
private:
    semaphore &sem;
public
    SemaphoreWrapper(semaphore &s)
    {
        sem = s;
        sem.lock();
    }

    ~SemaphoreWrapper()
    {
        sem.unlock();
    }
}

Then simply create an instance of SemaphoreWrapper within each function:

void func1()
{
    SemaphoreWrapper(global_semaphore);


    ...
}

The constructor and destructor of SemaphoreWrapper will take care of the lock/unlock functionality.

trojanfoe
  • 120,358
  • 21
  • 212
  • 242
1

You could write wrapper functions, e.g.:

int f1_locked(int x) {
  lock(..);
  int r=f1(x);
  unlock(..);
  return r;
}

With the preprocessor you may save some work.

EDIT

As someone stated, it would be better to move the implementations into library-internal functions, and present the wrappers to library users, e.g.:

// lib exports the wrapper:
int f1(int x) {
  lock(..);
  int r=f1_unlocked(x);
  unlock(..);
  return r;
}

// for library internal use only:
int f1_unlocked(int x) {
  ...
}

This has the additional advantage, that calls from and to lib internal functions do not need superfluous locking (which may be possible or not, this depends..), e.g.:

void f2_unlocked() {
  ...
  f1_unlocked();
  ...
}

void f2() {
  lock();
  f2_unlocked();
  unlock();
}
Frunsi
  • 7,099
  • 5
  • 36
  • 42
  • He specifically asked not to use unlock() as there are lots of return statements in these functions; point #2 in the question – Jaywalker Jan 12 '11 at 10:02
  • 2
    @Jaywalker, this solution doesn't modify the existing function, simply *wraps* it - hence there is only one return point and this will work for C as well, however it breaks point 1. – Nim Jan 12 '11 at 10:08
  • A wrapper function is needed, and RAII is called for to prevent explicit calling of the unlock function. A simple sentry object. – daramarak Jan 12 '11 at 10:11
  • see my answer for the preprocessor details... which should kinda fixes point 1 – smerlin Jan 12 '11 at 10:13
  • The functions should also be seemlessly callable after introducing the locks. In this case all the callers will have to update f1() to f1_locked which isn't feasible. – sharjeel Jan 12 '11 at 10:17
  • @sharjeel, but if you control the library, could you not move the code in `f1` to `f1_locked` and then the original `f1` simply is this wrapper which still delegates to the `f1_locked` - i.e. invert the function names in this solution. – Nim Jan 12 '11 at 10:20
  • @Frunsi Shouldn't it be r = f1_unlocked(x); in the EDIT? – batbrat Feb 10 '14 at 10:54
1

Don't.

You cannot go from single-threading to multi-threading by spewing a few locks here and there and hope for the best.

  1. Are you sure that there are no two functions that share a global variables ? C-functions are notorious for using statically allocated buffers.

  2. Mutexes are not normally reentrant. Therefore if you decorate f1 and f2 and one calls the other, you'll deadlock. You can, of course, use more expensive reentrant mutexes.

Multithreading is hard, at the best of times. It usually requires to understand and adjust the flow of execution.

I have a hard time imagining that throwing a few locks around would work for the best.

And that is obviously discounting the fact that if the functions were not crafted with MT in mind, they might well be slower (with all those mutex operations) and you won't, therefore, reap much benefit.

If you really need your semaphore, have the client lock it. He should know when to lock and when not to.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • Although this deviates from main conversation, i.e. decorators in C, your point is pretty valid for trying to make a single threaded library into multi-threaded library by putting a few locks. – sharjeel Jan 12 '11 at 10:53
  • The library is written in C which is called by Python bindings. It is working well in Python's multiple threads because of the GIL (Global Interpreter Lock) which solves concurrency issues at a very high price of efficiency. I've identified a few functions that need to be synchronized while others are pretty much harmless. – sharjeel Jan 12 '11 at 10:57
  • Minor point, but `errno` exists as a separate instance for each thread. – JeremyP Jan 12 '11 at 14:07
  • Oh and in the sentence *I have a hard imagining that throwing a few locks around would work for the best* you need to insert the word "on" after the fourth word :-) – JeremyP Jan 12 '11 at 14:09
  • @JeremyP: regarding 'on', I think I'll pass ;) – Matthieu M. Jan 12 '11 at 14:20
  • Although this is not a direct answer to the question, it is a very very good advice. I would like to add that even though the code might seem to run fine afterwards, you will probably end up chasing races for ages, as they never show their ugly faces until the "right time" is there. Redesign for multi-threading to avoid having that eerie voice in the back of your head while debugging ; "Might this be another race?" – daramarak Feb 10 '14 at 12:58
0

Decorators are strictly a language feature that provides syntactics sugar for the underlying semantics. The underlying semantics you can get in C++: just wrap the function appropriately; the syntactic sugar you can’t get.

A better alternative would be to create an appropriate code design that supports your use-case, e.g. by ineritance and the decorator pattern. This doesn’t necessarily imply class inheritance – the pattern can also be realized using class templates.

As for your specific use case, better alternatives have already been posted.

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
0

It seems to me that what you want to do is Aspect Oriented Programming (AOP). There are few AOP frameworks in C and C++, but from what I saw a few months ago I think the AspectC++ project provides a nice implementation of the AOP concepts. I have not tested it in production code though.

Luc Touraille
  • 79,925
  • 15
  • 92
  • 137