Locking access of entries in auth_server_static

[avaidyanatha]

Main Changes

• loadConfigFromParams is now called in a goroutine
• Mutex changed to RWMutex

Signed-off-by: avaidyanatha avaidyanatha@ucsb.edu

[avaidyanatha]

@demmer This should address your concerns with #4294!

[demmer]

Thanks for jumping on this @avaidyanatha -- I have some suggestions that might not have been super clear originally but I hope make sense here.

[demmer]

It seems to me the whole thing would be simplified if we just spun up a Timer instance to re-read the config periodically rather than trying to load it on-demand.

That way the regular accessor threads don't have to worry about checking the update time.

It might require adding a simple Stop() interface to the AuthServer so we can stop the timer at shutdown time, but that seems pretty straightforward IMO and would be an improvement.

[sougou]

I'm thinking we should do a uniform scheme that applies to all files. Currently, some are reloaded through a SIGHUP. So, I'd say let's do the SIGHUP handling for this definitely.

The next question: will a timer based reload be still be useful once we have SIGHUP? Not sure. What do you think?

[demmer]

Oh that's a good point.

In fact looking at the code, the mysql auth server itself handles SIGHUP already, so maybe this whole effort should be rolled back?

[avaidyanatha]

This makes sense, I've rolled back the auth rotation changes and changed the mutex back to a normal one.

[demmer]

Overall this is looking much better, but there are a couple of potentially dangerous locking patterns still in the code that I noticed.

We should be really careful to not hold locks while doing external activity like system calls and especially not while waiting for packets from clients.

[avaidyanatha]

@demmer Worked with Sugu on these changes - should be good now.

[demmer]

Looks great now. Thanks for tidying this up!