glassfish
  1. glassfish
  2. GLASSFISH-15351

FindBugs issue in security/jaspic-provider-framework

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.1_ms07
    • Fix Version/s: 3.1_ms08
    • Component/s: security
    • Labels:
      None

      Description

      monzillo: security/jaspic-provider-framework/src/main/java/com/sun/jaspic/config/factory/BaseAuthConfigFactory.java:141: UL_UNRELEASED_LOCK: com.sun.jaspic.config.factory.BaseAuthConfigFactory.getConfigProvider(String, String, RegistrationListener) does not release lock on all paths
      monzillo: security/jaspic-provider-framework/src/main/java/com/sun/jaspic/config/factory/BaseAuthConfigFactory.java:143: UL_UNRELEASED_LOCK: com.sun.jaspic.config.factory.BaseAuthConfigFactory.getConfigProvider(String, String, RegistrationListener) does not release lock on all paths

        Activity

        Hide
        kumarjayanti added a comment -

        To pacify findbugs the fix seems to be move the code into another method and restructure as follows :

        @Override
        public AuthConfigProvider
        getConfigProvider(String layer, String appContext,
        RegistrationListener listener) {
        AuthConfigProvider provider = null;
        if (listener == null) {
        rLock.lock();
        try

        { provider = getConfigProviderUnderLock(layer,appContext,null); }

        finally

        { rLock.unlock(); }

        } else {
        wLock.lock();
        try

        { provider = getConfigProviderUnderLock(layer,appContext,listener); }

        finally

        { wLock.unlock(); }

        }
        return provider;
        }

        Show
        kumarjayanti added a comment - To pacify findbugs the fix seems to be move the code into another method and restructure as follows : @Override public AuthConfigProvider getConfigProvider(String layer, String appContext, RegistrationListener listener) { AuthConfigProvider provider = null; if (listener == null) { rLock.lock(); try { provider = getConfigProviderUnderLock(layer,appContext,null); } finally { rLock.unlock(); } } else { wLock.lock(); try { provider = getConfigProviderUnderLock(layer,appContext,listener); } finally { wLock.unlock(); } } return provider; }
        Hide
        kumarjayanti added a comment -

        How bad is its impact? (Severity)
        minor

        How often does it happen? (Frequency)
        There is no real bug here as we understand, it is just that FindBugs complains about the code as incorrect locking pattern. So we have to restructure the code a bit to pacify findbugs.

        How much effort is required to fix it? (Cost)
        minor

        What is the risk of fixing it? (Risk)
        None

        When i tried to push back on fixing this, Bill shannon insisted that one of the goals was to ship with zero findbugs.

        Show
        kumarjayanti added a comment - How bad is its impact? (Severity) minor How often does it happen? (Frequency) There is no real bug here as we understand, it is just that FindBugs complains about the code as incorrect locking pattern. So we have to restructure the code a bit to pacify findbugs. How much effort is required to fix it? (Cost) minor What is the risk of fixing it? (Risk) None When i tried to push back on fixing this, Bill shannon insisted that one of the goals was to ship with zero findbugs.
        Hide
        kumarjayanti added a comment -

        moving to 3.2 since it was not approved for 3.1

        Show
        kumarjayanti added a comment - moving to 3.2 since it was not approved for 3.1
        Hide
        kumarjayanti added a comment -

        Fixed.

        Show
        kumarjayanti added a comment - Fixed.

          People

          • Assignee:
            kumarjayanti
            Reporter:
            kumarjayanti
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: