Is GenericObjectPools borrowObject Method thread safe?

Question

In this question Is GenericObjectPool<T> from commons.apache.org thread safe? It is mentioned that its thread safe .

Edited:But im having a situation in my multithreaded application that two threads are getting the same object from the pool at the same time.-This statement was wrong.

I moved the borrowObject to synchronize block and it solved my issue.

Has anyone faced this issue earlier?

Here is my code:

public static GenericObjectPool<IDocBuilderPool> documentBuilderPool = new GenericObjectPool(new DocumentPool());

static {
    documentBuilderPool.setMaxActive(1000);
    documentBuilderPool.setMaxWait(30000);
    documentBuilderPool.setMaxIdle(-1);

}
//method that returns document pool called by multiple threads .

public static IDocBuilderPool getDocumentPool() {

    return documentBuilderPool.borrowObject();
}

//The pool factory class
public class DocumentPool extends BasePoolableObjectFactory<ICollabrrDocument> {

    public DomDocumentPool() {
    }

    @Override
    public DomDocument makeObject() throws Exception {
        // TODO Auto-generated method stub
        return new DomDocument();
    }

    @Override
    public void activateObject(IDocBuilderPool obj) throws Exception {
        // TODO Auto-generated method stub
        super.activateObject(obj);
    }

    @Override
    public void destroyObject(IDocBuilderPool obj) throws Exception {
        // TODO Auto-generated method stub
        super.destroyObject(obj);

    }

    @Override
    public void passivateObject(IDocBuilderPool obj) throws Exception {
        // TODO Auto-generated method stub
        obj.release();
        super.passivateObject(obj);
    }

    @Override
    public boolean validateObject(IDocBuilderPool obj) {
        // TODO Auto-generated method stub
        return super.validateObject(obj);
    }
}




public class DomDocument implements IDocBuilderPool  {

private Document domDocument;
private DocumentBuilder documentBuilder;
private DocumentBuilderFactory documentBuilderFactory;

public HashMap<org.w3c.dom.Node, DOMElement> elementMap = new HashMap<org.w3c.dom.Node, DOMElement>();

public long threadID;

public DomDocument()  {


    setDomDocument();
    this.threadID = Thread.currentThread().getId();

}

public void setDomDocument() throws 
    this.documentBuilderFactory = DocumentBuilderFactory.newInstance();


        this.documentBuilderFactory.setNamespaceAware(true);
        this.documentBuilder = this.documentBuilderFactory.newDocumentBuilder();
        this.domDocument = this.documentBuilder.parse(new ByteArrayInputStream("<Root/>".getBytes()));


}

}

Can you post your test so we can reproduce it? It may be a bug in their code or your test. BTW Generic object pools can be more expensive than create new objects every time esp since Java 5.0. I use object pools but find that to make sure they are faster they need to be class specific object pool tuned to specific use cases. — Peter Lawrey, Aug 08 '13 at 13:27
All the public methods of the class are synchronized so it looks ok (a task is scheduled from the constructor but it is unlikely to be a problem). So the problem is possibly in your code... — assylias, Aug 08 '13 at 13:31
Hi assylias ., Is the task scheduled as per the eviction policy of the genericobject pool .Any way i have not enabled it. Please check the code above. — Bijesh CHandran, Aug 08 '13 at 13:43
@assylias . Sorry for the confusion. Actually the problem was in my object creation. I was creating a documentbuilderfactory,documentbuilder and a dom document(db.parse()) with inside the constructor which are not thread safe. — Bijesh CHandran, Aug 09 '13 at 08:47
Making DomDocument -->setDomDocumentObject() synchronized solved my issue . — Bijesh CHandran, Aug 09 '13 at 08:58

score 2 · Accepted Answer · answered Aug 08 '13 at 18:57

2

The documentation of PoolableObjectFactory states:

PoolableObjectFactory must be thread-safe.

Looking at your code, the only thing that could be thread unsafe is the call to obj.release();. This is possibly where your problem is.

Apart from that all looks ok...

answered Aug 08 '13 at 18:57

assylias

321,522
82
660
783

obj.release() is an instance call,that works only on that particular object. – Bijesh CHandran Aug 09 '13 at 08:35
1

@BijuCNair Actually I gave you some BS - not sure what code I was looking at, but [this implementation v1.6 of GenericObjectPool](http://commons.apache.org/proper/commons-pool/api-1.6/src-html/org/apache/commons/pool/impl/GenericObjectPool.html#line.192) is **not** all synchronized. In particular, the `borrowObject` method (line 1058), which returns the new objects, uses complex synchronization and it is difficult to say at first glance if it is correct. There could well be a bug in it (I don't know). – assylias Aug 09 '13 at 08:44

Is GenericObjectPools borrowObject Method thread safe?

1 Answers1

Linked