Re[2]: rlm_detail bug when "locking = yes"
Nicolas wrote:
we need close outfd right before "return RLM_MODULE_FAIL;" at line 236.
.... or get the fixes from CVS head which solve many other problems in rlm_detail. (for example file renamed or unlinked by another program)
Everyone will save a considerable amount of time if you could backport the changes from CVS to 1.1.2, run tests to check the new code, and send unified diff to the list.
i try to examine snapshot-20060616 btw, there are old bug in rlm_detail.c. 279: if ((outfd = open(buffer, O_WRONLY | O_APPEND | O_CREAT, 280: inst->detailperm)) < 0) { 281: radlog(L_ERR, "rlm_detail: Couldn't open file %s: %s", 282: buffer, strerror(errno)); 283: return RLM_MODULE_FAIL; 284: } 286: /* 287: * If we fail to aquire the filelock in 80 tries 288: * (approximately two seconds) we bail out. 289: */ 290: if (inst->locking) { 291: lseek(outfd, 0L, SEEK_SET); 292: if (rad_lockfd_nonblock(outfd, 0) < 0) { 293: close(outfd); 294: tv.tv_sec = 0; 295: tv.tv_usec = 25000; 296: select(0, NULL, NULL, NULL, &tv); 297: lock_count++; 298: continue; 299: } open() executed once. but while we failed to get lock at first try, close(outfd) executed and all the subsequent attempts to lock file fails i comment line 293 out. After this i havn't got any messages like "Error: rlm_detail: Failed to aquire filelock for /var/log/radius/radacct/detail, giving up" in log! so now rlm_detail with "lock=yes" and radrelay all works fine. Mike.
Michael Chernyakhovsky wrote:
open() executed once. but while we failed to get lock at first try, close(outfd) executed and all the subsequent attempts to lock file fails
i comment line 293 out.
I don't understand why this helps. We open the file at the beginning of the do..while() loop, so it should be fine. BTW if you remove the close() line 293 you're leaking one file descriptor each time the lock fails. -- Nicolas Baradakis
open() executed once. but while we failed to get lock at first try, close(outfd) executed and all the subsequent attempts to lock file fails
i comment line 293 out.
I don't understand why this helps. We open the file at the beginning of the do..while() loop, so it should be fine.
err. sorry. source of snapshot are like to freeradius-1.1.2 (rlm_detail.c,v1.37.2.1.2.1 2006/04/19). i miss that open() was moved inside loop :(
BTW if you remove the close() line 293 you're leaking one file descriptor each time the lock fails. i use stable 1.1.2, not snapshot. there open() called before do-while loop in this version. close() executed every iteration. so i have to comment close() in block if (rad_lockfd_nonblock(outfd, 0) < 0) { close(outfd); tv.tv_sec = 0; tv.tv_usec = 25000; select(0, NULL, NULL, NULL, &tv); lock_count++; } for correct work.
sorry again. Mike.
participants (2)
-
Michael Chernyakhovsky -
Nicolas Baradakis