move metric initialisation upfront connection creation to avoid potential npe

[philnate]

We have a setup where we regularly create a new Memcached client to verify memcached is still running. Now what happens if you totally cut down your network (no LAN & WLAN). You'll get a NPE within MemcachedConnection as the first connection try fails, and then the failure handling tries again trying to increment a metric, which at this time isn't init'd.

java.lang.NullPointerException
    at net.spy.memcached.MemcachedConnection.queueReconnect(MemcachedConnection.java:714)
    at net.spy.memcached.MemcachedConnection.createConnections(MemcachedConnection.java:241)
    at net.spy.memcached.MemcachedConnection.<init>(MemcachedConnection.java:174)
    at net.spy.memcached.DefaultConnectionFactory.createConnection(DefaultConnectionFactory.java:196)
    at net.spy.memcached.MemcachedClient.<init>(MemcachedClient.java:207)

This alone wouldn't be too bad if during init a Selector was opened already, which then isn't closed any more. So on mac each time the Selector is opened 2 PIPEs and 1 KQUEUE are created, this leads eventually to a create AF_UNIX socket: Too many open files in system error making the whole system unresponsive.

  public MemcachedConnection(final int bufSize, final ConnectionFactory f,
      final List<InetSocketAddress> a, final Collection<ConnectionObserver> obs,
      final FailureMode fm, final OperationFactory opfactory) throws IOException {
    //...
    selector = Selector.open();

I've fixed the NPE by moving the metric init to an earlier place, but maybe some try catch around the whole init method would be better to close the all potentially opened handles before bubbling the exception.

[philnate]

@daschl any plans to merge this into the main branch?

[daschl]

@philnate yeah I think we can merge something like this, but we should also see how we can address the NPE, right?

[philnate]

Sounds good. Well my understanding was that the ordering of the commands where wrong. So the proposed solution would be to make sure the metrics are initialized before the connection is tried to be establised. What else should be addressed regarding the NPE, that looked like the main reason to me?

[michaelkwan]

Is there no plans to fix this?