Printable View
hi,
Apparently MinGW hasn't got localtime_r, and I was concerned about the non thread safeness of using localtime.
I've added localtime_r to my code with the following;
Is that actually safe, or is it merely minimising potential problems?Code:#ifdef Win32
inline struct tm* localtime_r (const time_t *clock, struct tm *result) {
if (!clock || !result) return NULL;
memcpy(result,localtime(clock),sizeof(*result));
return result;
}
#endif
nik
that's not thread safe at all, you need some kind of synchronisation object to protect it.
regards,
I thought as much...
I think I'd prefer writing a thread safe localtime function than just putting a mutex lock around the function above - this seems like reinventing the wheel though. Does anyone know of some localtime_r source for windows?
nik
What is not thread-safe???? localtime ???? Not even from the MultiThreaded version of CRT?????Quote:
Originally Posted by Darka
Regards,
Usman.
Well, you are not using any member var as far as i see, and also memcpy (you need to use MT CRT) is threadsafe. The only remaining function is localtime (i guess it comes from CRT) and from MT CRT i suppose its implemented in a Thread-safe way. If all of these conditions are true, you dont need to syn the call to your local version of localtime_r.Quote:
Originally Posted by nik_cain
Hope this helps,
Regards,
Usman.
memcpy is not threadsafe
neither is localtime, just because you are using the MT version of the CRT does not mean everything is threadsafe.
actually, let me explain a little more.
memcpy is only threadsafe when the source and destination addresses are only accessible from a single thread, if they are accessible from other threads, then it is not thread safe.
How else can you make memcpy thread-safe. Is it possible for *any* function to know if the pointer passed into as in-param is also held by another thread????? And if thats the case then any thread-safe is not threadsafe according to you. How do u define a function to be Threadsafe then???? if you think memcpy is not.Quote:
Originally Posted by Darka
Usman.
Hi Usman,
You are missing the point, the memcpy() function is threadsafe in that it can be used from many threads at the same time without any problems. However, the pointers you pass to memcpy() also need to be threadsafe, else you could end up with this...
To make a function threadsafe, you need to ensure the parameters passed into it are safe as well as the function itself.Code:Thread 1 enters memcpy( ) and is suspended by the OS, just before the actual data is copied to the passed in pointer.
Thread 2 deletes the object pointed to by one of the parametes that Thread 1 passed to memcpy() and is then suspended.
Thread 1 wakes up and continues copying the data to the deleted location.
Also...
no, it is up to the developer to know if the parameters being passed to the function are protected.Quote:
Is it possible for *any* function to know if the pointer passed into as in-param is also held by another thread?????
Then is it possible to write a Thread Safe function???? Even if there is a syn object inside memcpy so only one thread enters and excecutes the code the condition you mentioned can still occur. So then we dont have any Thread Safe function in CRT??? There are also bad ways of doing things, bypass the right procedure, and that situation you mentioned is one of them, the problem of the programmer not of the function. Secondly that postQuote:
Originally Posted by Darka
http://groups-beta.google.com/group/...cb818ecb0bc731
seems to say that localtime is also Thread safe, as the return sturct is per thread not per system or process.
Regards,
Usman.
Hi Usman,
yes, so long as you protect any shared part of that function (including the parameters passed into it) if they are shared accross multiple threads.Quote:
Then is it possible to write a Thread Safe function????
correct, the sync object should be to protect the parameters to the localtime_r function, not within the actual memcpy call.Quote:
Even if there is a syn object inside memcpy so only one thread enters and excecutes the code the condition you mentioned can still occur.
regards,
so... going back to my original question - if anyone knows a threadsafe windows implementation of localtime I can use (while not having access to localtime_r), I'd be very grateful. I can protect my call to localtime in the meantime, but it's an unneccessary potential bottleneck as far as I can see.
nik
you only need to protect it if the parameters you are passing to it are accessible from more than one thread, or if the function itself is callable from multiple threads. If not, you do not need to protect them, but should then use localtime() directly.
If they need protecting then you MUST protect them and it is then not an unneccessary bottle neck.
regards,
sorry to flog ol' neddy,
I have the situation where localtime is called from a thread created by a certain class. It's more than likely that an application will have created several instances of that class, and so it's possible that several threads of the same process will call localtime. The structs tm result and time_t clock are local to each thread, and so don't need protecting.
An entirely threadsafe localtime would mean I don't even have to think about what's going on, and the application could have as many instances of that class as they like. However, protecting the call to the existing non-threadsafe localtime ( ie;
would mean that all instances of that class would wait for global_mutex, and hence the bottleneck.Code:#ifdef Win32
boost::mutex global_mutex;
inline struct tm* localtime_r (const time_t *clock, struct tm *result) {
boost::mutex::scoped_lock lock(global_mutex);
if (!clock || !result) return NULL;
memcpy(result,localtime(clock),sizeof(*result));
return result;
}
#endif
Is that true? It makes sense to me, but I'd like to be 100% on this...
nik
Jumping in late here.... I see a couple of things:
1) Do you really need to use a mutex here? If all you care about is synchronization across threads (vs. across processes), won't a cs do the job for you?
2) Is the function you posted a method to a class?
3) Why lock on a global level? You can lock only on a per-instance level or better yet, on a per-method level. Make the mutex (or cs) an instance member of the class and just lock it for the minimum time needed.
4) Does the const time_t *clock variable need to be passed in to this method? Can it be local instead?
5) Consider swapping the boost lock statement with the parameter check statement. That way, the code only locks when necessary.
Arjay
Good points Arjay,
Mutex's are MUCH slower than Critical sections.
Both mutexes and critical sections will slow down your code.
Here is some code I have tested which gives the safety of a mutex
without the overhead of using the OS level code.
I found that this improves the performance of my multithreaded server by about 500%!
myfunc() // protects the non-reentrant functions like malloc()
{
static int protector=0;
protector++;
while(protector>1){
protector--;
sleep(1);// sleep for milsec
protector++;
}
protected_function();
protector--;
}
it's just a shame then that your code is not thread safe in the slightest....
I think it is theoretically threadsafe because the protector variable will
never loose count of the increments and decrements assuming that it is
excuted as a single cpu instruction. I have also done all types of testing to
disprove it.
Your increments/decrements are not threadsafe, they are not atomic single instructions, you could use InterlockedIncrement() / InterlockedDecrement() instead though.
not even that would make the code thread safe though as you have no idea which thread acquired the lock, if you need a high speed locking mechanism, have a look at spin counts, or even "Jeffery Richters" COptex class, which is a high speed cross process critical section.
regards,
In my application which runs on Windows,linux,os/2 I have found that this works.
I think I will have to analyse it futher as this was tested only on single cpu machines.
I believe that if you have increments/decrements as atomic single instructions
(which you can ensure by using assembly) this works, as long as it is
not important which thread has aquired the lock.
I am probably mistaken in this.
Thanks darka for pointing out other possibilities.
In any case I needed a mechanism that is not platform dependent.
That is the reason I am so keen on this.
Thanks for replies
You can't have platform independence when you use assembler - and I can't see why your so keen on your idea when it is not thread safe ?
May be I am so dense I cant see where my code fails .. Can you explain?
You are not dense (multithread programming can be a git) but the problem is in that you increments/decrements are not atomic.
Changing this to assembly language breaks the platform independence that you would like, I assume that Linux or whatever other OS you are working with has interlocked increment/decrement functions too (probably called something else).
Only by using Interlocked versions of increment / decrement are you making them atomic.
You could simply use a critical section (and you can get cross platform versions) instead though. Critical sections are pretty fast, especially when compared with Mutexes.
regards,
I got it now - the increment/decrement operations may not be compiled into
single instructions so it breaks my code.
Initially I was using critical sections for this, but since porting to linux
could not find something equivalent. So started using mutexes and looking
for alternatives.
Thanks!
Hi Kdv,
exactly..Quote:
I got it now - the increment/decrement operations may not be compiled into
I don't write cross platform code so cannot advise on that.
regards,
[Responding here in case other folks see this and don't read your later responses] Sure, synchronization objects take time to execute, but having reliable, robust code that may be slower is preferable to faster code that is susceptible to corruption.Quote:
Originally Posted by kdv
Hi i recently had the same problem and came up with this solution ..
// Global monitor object for the localtime function
CCriticalSection oLockLocaltime;
void MPSLocaltime(const time_t& ttTime,struct tm* ptrTM)
{
if(! ptrTM)
return;
CSingleLock oLock(&oLockLocaltime);
oLock.Lock();
struct tm* ptrTmLoc;
ptrTmLoc = localtime(&ttTime);
memcpy(ptrTm,ptrTmLoc,sizeof(struct tm));
oLock.Unlock();
}
This should do the trick ..
I didn't used CCriticalSection but some lock object for linux i wrote. I is based on the pthread lib but it basically works exactly like the CCriticalSeection/CSingle lock combo under Win32. I know because when i compile the code inside windows i simply change the base class to CCriticalSection/CSingleLock.
Greetings ..