addWebElement in WebApplication can lead to a filter/servlet already registered exception [PAXWEB-485]

[ops4j-issues]

Christian Berger created PAXWEB-485

I'm using a highly concurrent OSGi application (Declarative Services) and it took me some time to figure out this particular bug. In my case there was a parallel http context registration just between these two locks:

public void addWebElement( final WebElement webElement )
    {
        NullArgumentException.validateNotNull( webElement, "Registerer" );
        m_webElementsLock.writeLock().lock();
        try {
            m_webElements.add( webElement );
        } finally {
            m_webElementsLock.writeLock().unlock();
        }
// http context registration which wasn't available yet

        m_httpServiceLock.readLock().lock();
        try
        {
            registerWebElement( webElement );
        }
        finally
        {
            m_httpServiceLock.readLock().unlock();
        }
    }

I guess for me it's enough if you change the order of the code so that the registration comes first:

public void addWebElement( final WebElement webElement )
    {
        NullArgumentException.validateNotNull( webElement, "Registerer" );

        m_httpServiceLock.readLock().lock();
        try
        {
            registerWebElement( webElement );
        }
        finally
        {
            m_httpServiceLock.readLock().unlock();
        }
// http context registration which wasn't available yet

        m_webElementsLock.writeLock().lock();
        try {
            m_webElements.add( webElement );
        } finally {
            m_webElementsLock.writeLock().unlock();
        }
}

Another benefit to do the registration first is for that case when a registration fails. This avoids adding an element to webElements whose registration goes wrong.

Nevertheless, this solves not the problem that there is still some time for concurrent modification between the two locks. IMHO it would be better to have just one lock for both webElements and httpservice.

---

Affects: 3.0.0.M2 Fixed in: 3.0.0.M3, 3.0.0 Votes: 0, Watches: 1

[ops4j-issues]

Achim Nierbeck commented

I'm gonna take a look at it.
Though to find such regression in future, you don't happen to have an example application which could be used by the itests?
Would be really nice if you could provide us such an example application so we can make sure it won't be an issue in the future again.

[ops4j-issues]

Christian Berger commented

@Achim: the mentioned application is closed source at the moment. However, our management board is thinking about a version for testing purpose only. I'll inform you as soon as a decision has been made.

[ops4j-issues]

Achim Nierbeck commented

Branch: refs/heads/master
Home: https://github.com/ops4j/org.ops4j.pax.web
Commit: abd3d89f3f4baf38ba57623b1215b27349a324aa
https://github.com/ops4j/org.ops4j.pax.web/commit/abd3d89f3f4baf38ba57623b1215b27349a324aa
Date: 2013-02-27 (Wed, 27 Feb 2013)