On 11/04/14 12:24, Alan DeKok wrote:
Stefan Winter wrote:
Whoa. Why would that be a configuration error?
Because we can't guarantee that the writes occur properly.
I have multiple instances of rlm_sql_null, all writing different SQL statements, and all writing into the same file for forwarding to the SQL server by radsqlrelay.
Which doesn't work with POSIX locks, as you pointed out. It might work accidentally, but different threads writing to the same file will cause problems. One thread closing a file descriptor will cause another thread to magically lose its lock.
I think it's important to note this *will* be working for 2.2.x on Linux because the locking #ifdef will end up using flock() which has saner, per-fd semantics. That said: I don't see why your changes would cause semantic problems here. Since all rlm_sql instances use the logging api, surely they'll all interact just fine with >1 instance writing to the same name? I am rather concerned about the locking overhead of the new approach though; my gut feeling tells me a double mutex acquire/release will likely be more expensive than the open()/close() calls, but hopefully I'm wrong.