Menu

#8 Memory leak in LocalIdentityRepository

Unstable (example)
open
nobody
None
5
2016-03-13
2015-01-14
Rob Moore
No

We are using Spring Integration in our product and are running into a memory leak that appears to be related to JSch. We've identified an issue in LocalIdentityRepository where Identity objects are being added repeatedly to the repository even though they are already present.

LocalIdentityRepository has the following code:

  public synchronized void add(Identity identity) {
    if(!identities.contains(identity)) {
      identities.addElement(identity);
    }
  }

  public synchronized boolean add(byte[] identity) {
    try{
      Identity _identity =
        IdentityFile.newInstance("from remote:", identity, null, jsch);
      identities.addElement(_identity);
      return true;
    }
    catch(JSchException e){
      return false;
    }
  }

The field identities is a Vector and contains() uses equals() to compare the t​he two objects. As there's no declared method in IdentityFile, the default Object equals method is used which simply compares the two objects to see if they are the same instance. This will fail in most situations.

We believe that there needs to an implementation of equals and hashCode added to IdentityFile.

I'm attaching a possible patch which is based on the logic in remove(). Specifically, it uses the key blob to determine whether an Identity is in the repo and should be removed.

  synchronized void remove(Identity identity) {
    identities.removeElement(identity);
  }

  public synchronized boolean remove(byte[] blob) {
    if(blob == null) return false;
    for(int i=0; i<identities.size(); i++) {
      Identity _identity = (Identity)(identities.elementAt(i));
      byte[] _blob = _identity.getPublicKeyBlob();
      if(_blob == null || !Util.array_equals(blob, _blob))
        continue;
      identities.removeElement(_identity);
      _identity.clear();
      return true;
    }
    return false;
  }
1 Attachments

Discussion

  • Rob Moore

    Rob Moore - 2015-01-14

    Adding corrected version of patch. Was treating incoming object as blob rather than IdentityFile in equals().

     
  • Alexsey

    Alexsey - 2016-03-13

    Thank you! I also got out of memory leaks, I hope your decision will help.

     

Log in to post a comment.