Issue 6004 - idletimeout may be ignored #6005
Conversation
| slapi_eq_repeat_rel(check_idletimeout, NULL, | ||
| slapi_current_rel_time_t(), | ||
| MILLISECONDS_PER_SECOND); |
There was a problem hiding this comment.
I wonder if it's not too often... (scanning all of the connections every second)
Probably, it makes sense to see that there is no performance drop in situations with many user connections.
There was a problem hiding this comment.
I doubt we will notice anything:
During stress test the listener threads are walking all the active connections every tenth of ms (or even faster) to build and check the polling table so walking it every second should have an impact below 0.01%
ldap/servers/slapd/daemon.c
Outdated
| if (pthread_mutex_trylock(&(c->c_mutex)) == EBUSY) { | ||
| continue; | ||
| } | ||
| if (c->c_state != CONN_STATE_FREE && !c->c_gettingber && |
There was a problem hiding this comment.
This test is looking the opposite (!) of the test above (that skip this connection). It would be good to have a single macro/function.
There was a problem hiding this comment.
Added an inline function
| /* | ||
| * slapi_eq_repeat_rel callback that checks that idletimeout has not expired. | ||
| */ | ||
| void |
There was a problem hiding this comment.
I like this approach but I wonder why the previous approach is no longer working.
In slapd_deamon we are looping to call setup_pr_read_pds with a timeout (1s) on poll.
Is idletime not longer tested because this function is no longer used ?
There was a problem hiding this comment.
setup_pr_read_pds is still called (by ct_list_thread) at least every seconds but it does not test for idle timeout !
If you grep c_idletimeout in previous code, you see that the only place it is tested is in handle_pr_read_ready
and ct_list_thread does not call handle_pr_read_ready if the poll timed out
Problem: idletimeout is still not handled when binding as non root (unless there are some activity
on another connection)
Fix:
Add a slapi_eq_repeat_rel handler that walks all active connection every seconds and check if the timeout is expired.
Note about CI test:
Notice that idletimeout is never enforced for connections bound as root (i.e cn=directory manager).
Issue #6004
Reviewed by: @droideck, @tbordaz (Thanks!)