grizzly
  1. grizzly
  2. GRIZZLY-1568

Require more identifiers other than Name of NetworkListener for GrizzlyService.removeNetworkProxy.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.4
    • Fix Version/s: 2.3.6
    • Component/s: None
    • Labels:
      None

      Description

      Currently GrizzlyService.removeNetworkProxy takes NetworkListener.name (id) as parameter for removing.

           * Remove the proxy from our list of proxies by id.
           * @return <tt>true</tt>, if proxy on specified port was removed,
           *         <tt>false</tt> if no proxy was associated with the port.
           */
          public boolean removeNetworkProxy(String id) {
              NetworkProxy proxy = null;
              for (NetworkProxy p : proxies) {
                  if (p instanceof GrizzlyProxy) {
                      GrizzlyProxy grizzlyProxy = (GrizzlyProxy) p;
                      if (grizzlyProxy.networkListener != null &&
                              grizzlyProxy.networkListener.getName() != null &&
                              grizzlyProxy.networkListener.getName().equals(id)) {
                          proxy = p;
                          break;
                      }
                  }
              }
      

      There are two issues:

      1) If more than one NetworkListener instances with the same name were added by GrizzlyService.createNetworkProxy(NetworkListener listener) (that is valid operation), then removeNetworkProxy can delete only 1 of them.

      2) And also we have no way to delete a specific NetworkListener among those with same name.

      Reson:

      JMS might create more NetworkListener instances which are bound to the same LazyServiceInitializer for inbound requests handling, so the listerners must have the same name to achive this according to current Grizzly implementation (Refer to com.sun.enterprise.v3.services.impl.ServiceInitializerFilter.handleAccept(final FilterChainContext ctx) for details).

      Requirements:

      1) Add identifier id for NetworkListener. For example, new NetworkListener(String id).
      2) Add a method removeNetworkProxy(String name, String id) to GrizzlyService, which will remove the listener with both name and id matched.
      3) Change method GrizzlyService.removeNetworkProxy(String id) to removeNetworkProxy(String name), which should remove all listeners with the name.

        Issue Links

          Activity

          Hide
          oleksiys added a comment -

          The NetworkListener name is an id, it has to be unique. So most probably there is a bug in GrizzlyService.createNetworkProxy(NetworkListner listener), we should not allow to create NetworkProxies with the same name.
          We define the name in NetworkListener entitity like:

          ....
          @Attribute(required = true, key = true)
          ....
          

          so what we really need is to fix GrizzlyService.createNetworkProxy(NetworkListener listener)

          Show
          oleksiys added a comment - The NetworkListener name is an id, it has to be unique. So most probably there is a bug in GrizzlyService.createNetworkProxy(NetworkListner listener), we should not allow to create NetworkProxies with the same name. We define the name in NetworkListener entitity like: .... @Attribute(required = true , key = true ) .... so what we really need is to fix GrizzlyService.createNetworkProxy(NetworkListener listener)
          Hide
          David Zhao added a comment - - edited

          GF JMS had created multiple NetworkListeners with same name for long time, since V3. If you restrict GrizzlyService.createNetworkProxy(NetworkListener listener) to not allow duplicate name, then it will break JMS functionality unless com.sun.enterprise.v3.services.impl.ServiceInitializerFilter.handleAccept(final FilterChainContext ctx) is also changed alone with your proposal on createNetworkProxy.

          The reason is for GF JMS, we need creating multiple NetworkListeners which invoke the same LazyServiceInitializer. But the NetworkListener and LazyServiceInitializer are referenced by name. So it is not doable to bind the same LazyServiceInitializer to multiple NetworkListeners with different names by current Grizzly API (e.g. class ServiceInitializerFilter).

              @Override
              public NextAction handleAccept(final FilterChainContext ctx) throws IOException {
                  final NIOConnection nioConnection = (NIOConnection) ctx.getConnection();
                  final SelectableChannel channel = nioConnection.getChannel();
                  if (targetInitializer == null) {
                      synchronized (LOCK_OBJ) {
                          if (targetInitializer == null) {
                              LazyServiceInitializer targetInitializerLocal = null;
                              for (final ActiveDescriptor<?> initializer : initializerImplList) {
                                  String listenerName = listener.getName();
                                  String serviceName = initializer.getName();
                                  if (serviceName != null && listenerName.equalsIgnoreCase(serviceName)) {
                                      targetInitializerLocal = (LazyServiceInitializer) locator.getServiceHandle(initializer).getService();
                                      break;
                                  }
                              }
          
                              if (targetInitializerLocal == null) {
                                  logger.log(Level.SEVERE, "NO Lazy Initialiser implementation was found for port = {0}",
                                          String.valueOf(listener.getPort()));
                                  nioConnection.close();
          
                                  return ctx.getStopAction();
                              }
                              if (!targetInitializerLocal.initializeService()) {
                                  logger.log(Level.SEVERE, "Lazy Service initialization failed for port = {0}",
                                          String.valueOf(listener.getPort()));
          
                                  nioConnection.close();
          
                                  return ctx.getStopAction();
                              }
                              
                              targetInitializer = targetInitializerLocal;
                          }
                      }
                  }
          

          It would be expected that the above code (condition) can be changed

          from:

              if (serviceName != null && listenerName.equalsIgnoreCase(serviceName)) {
          

          to:

              if (serviceName != null && (listenerName.equalsIgnoreCase(serviceName) || serviceName.equals(listener.getServiceName())) {
          
          Show
          David Zhao added a comment - - edited GF JMS had created multiple NetworkListeners with same name for long time, since V3. If you restrict GrizzlyService.createNetworkProxy(NetworkListener listener) to not allow duplicate name, then it will break JMS functionality unless com.sun.enterprise.v3.services.impl.ServiceInitializerFilter.handleAccept(final FilterChainContext ctx) is also changed alone with your proposal on createNetworkProxy. The reason is for GF JMS, we need creating multiple NetworkListeners which invoke the same LazyServiceInitializer. But the NetworkListener and LazyServiceInitializer are referenced by name. So it is not doable to bind the same LazyServiceInitializer to multiple NetworkListeners with different names by current Grizzly API (e.g. class ServiceInitializerFilter). @Override public NextAction handleAccept( final FilterChainContext ctx) throws IOException { final NIOConnection nioConnection = (NIOConnection) ctx.getConnection(); final SelectableChannel channel = nioConnection.getChannel(); if (targetInitializer == null ) { synchronized (LOCK_OBJ) { if (targetInitializer == null ) { LazyServiceInitializer targetInitializerLocal = null ; for ( final ActiveDescriptor<?> initializer : initializerImplList) { String listenerName = listener.getName(); String serviceName = initializer.getName(); if (serviceName != null && listenerName.equalsIgnoreCase(serviceName)) { targetInitializerLocal = (LazyServiceInitializer) locator.getServiceHandle(initializer).getService(); break ; } } if (targetInitializerLocal == null ) { logger.log(Level.SEVERE, "NO Lazy Initialiser implementation was found for port = {0}" , String .valueOf(listener.getPort())); nioConnection.close(); return ctx.getStopAction(); } if (!targetInitializerLocal.initializeService()) { logger.log(Level.SEVERE, "Lazy Service initialization failed for port = {0}" , String .valueOf(listener.getPort())); nioConnection.close(); return ctx.getStopAction(); } targetInitializer = targetInitializerLocal; } } } It would be expected that the above code (condition) can be changed from: if (serviceName != null && listenerName.equalsIgnoreCase(serviceName)) { to: if (serviceName != null && (listenerName.equalsIgnoreCase(serviceName) || serviceName.equals(listener.getServiceName())) {
          Hide
          David Zhao added a comment -

          Please notify me when the fix is integrated into GF. I will need to update JMS module accordingly to avoid broken.

          Show
          David Zhao added a comment - Please notify me when the fix is integrated into GF. I will need to update JMS module accordingly to avoid broken.
          Hide
          oleksiys added a comment -

          I've commited the change to glassfish trunk.
          Now instead of having protocol.name == "light-weight-listener", you can set network-listener.type="proxy" and protocol.name=$

          {actual-service-name}

          let me know if it works for you.

          Show
          oleksiys added a comment - I've commited the change to glassfish trunk. Now instead of having protocol.name == "light-weight-listener", you can set network-listener.type="proxy" and protocol.name=$ {actual-service-name} let me know if it works for you.
          Hide
          oleksiys added a comment -

          David, pls. update the issue once you have time to test the fix.
          thx.

          Show
          oleksiys added a comment - David, pls. update the issue once you have time to test the fix. thx.
          Hide
          David Zhao added a comment -

          Thanks. The fix works for me.

          Show
          David Zhao added a comment - Thanks. The fix works for me.
          Hide
          oleksiys added a comment -

          fixed

          Show
          oleksiys added a comment - fixed

            People

            • Assignee:
              Unassigned
              Reporter:
              David Zhao
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: