Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4817

Threads get blocked due to unnecessary synchronization in OgnlRuntime

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.32, 2.5.12
    • Fix Version/s: 2.3.34, 2.5.13
    • Component/s: Core
    • Labels:
      None

      Description

      In multi-threaded scenario, due to unnecessary synchronization in invokeMethod method in OgnlRuntime class all threads except the first go to BLOCKED state.

      public static Object invokeMethod(Object target, Method method, Object[] argsArray)
          throws InvocationTargetException, IllegalAccessException
        { 
      .......
       synchronized (method) {
            if ((_methodAccessCache.get(method) == null) || (_methodAccessCache.get(method) == Boolean.TRUE))
            {
              syncInvoke = true;
            }
      .......
          }
      

      Because syncInvoke becomes true for the first thread irrespective of whether the method is public or not, all other threads go to block state till the first thread leaves the method invocation synchronization block.

      Attached threadDump of the blocked threads waiting to lock the monitor 0x00000006c690e5b8 even though the method invoked by the action is public.

      
      "_###_Thread-1499929571461_###_http-nio-8443-exec-20" #374 daemon prio=5 os_prio=0 tid=0x00007f830513a000 nid=0x49c1 waiting for monitor entry [0x00007f7e9c7c2000]
         java.lang.Thread.State: BLOCKED (on object monitor)
      	at ognl.OgnlRuntime.invokeMethod(OgnlRuntime.java:826)
      	- waiting to lock <0x00000006c690e5b8> (a java.lang.reflect.Method)
      	at ognl.OgnlRuntime.callAppropriateMethod(OgnlRuntime.java:1294)
      	at ognl.ObjectMethodAccessor.callMethod(ObjectMethodAccessor.java:68)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethodWithDebugInfo(XWorkMethodAccessor.java:117)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethod(XWorkMethodAccessor.java:108)
      	at ognl.OgnlRuntime.callMethod(OgnlRuntime.java:1370)
      	at ognl.ASTMethod.getValueBody(ASTMethod.java:91)
      	at ognl.SimpleNode.evaluateGetValueBody(SimpleNode.java:212)
      	at ognl.SimpleNode.getValue(SimpleNode.java:258)
      	at ognl.Ognl.getValue(Ognl.java:467)
      	at ognl.Ognl.getValue(Ognl.java:431)
      	at com.opensymphony.xwork2.ognl.OgnlUtil$3.execute(OgnlUtil.java:352)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.compileAndExecuteMethod(OgnlUtil.java:404)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.callMethod(OgnlUtil.java:350)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeAction(DefaultActionInvocation.java:430)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeActionOnly(DefaultActionInvocation.java:290)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:251)
      	at com.opensymphony.xwork2.interceptor.DefaultWorkflowInterceptor.doIntercept(DefaultWorkflowInterceptor.java:168)
      	.........
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:245)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	..........
      	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141)
      	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
      	at org.apache.catalina.authenticator.SingleSignOn.invoke(SingleSignOn.java:240)
      	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
      	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:502)
      	at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1132)
      	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:684)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1533)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1489)
      	- locked <0x00000006c5249d98> (a org.apache.tomcat.util.net.NioChannel)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
      	at java.lang.Thread.run(Thread.java:748)
      
      "Thread-1499929605769_###_Thread-1499929605766_###_http-nio-8443-exec-19" #373 daemon prio=5 os_prio=0 tid=0x00007f8305138000 nid=0x49c0 waiting for monitor entry [0x00007f7e9c8c3000]
         java.lang.Thread.State: BLOCKED (on object monitor)
      	at ognl.OgnlRuntime.invokeMethod(OgnlRuntime.java:826)
      	- waiting to lock <0x00000006c690e5b8> (a java.lang.reflect.Method)
      	at ognl.OgnlRuntime.callAppropriateMethod(OgnlRuntime.java:1294)
      	at ognl.ObjectMethodAccessor.callMethod(ObjectMethodAccessor.java:68)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethodWithDebugInfo(XWorkMethodAccessor.java:117)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethod(XWorkMethodAccessor.java:108)
      	at ognl.OgnlRuntime.callMethod(OgnlRuntime.java:1370)
      	at ognl.ASTMethod.getValueBody(ASTMethod.java:91)
      	at ognl.SimpleNode.evaluateGetValueBody(SimpleNode.java:212)
      	at ognl.SimpleNode.getValue(SimpleNode.java:258)
      	at ognl.Ognl.getValue(Ognl.java:467)
      	at ognl.Ognl.getValue(Ognl.java:431)
      	at com.opensymphony.xwork2.ognl.OgnlUtil$3.execute(OgnlUtil.java:352)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.compileAndExecuteMethod(OgnlUtil.java:404)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.callMethod(OgnlUtil.java:350)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeAction(DefaultActionInvocation.java:430)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeActionOnly(DefaultActionInvocation.java:290)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:251)
      	........
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:245)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	.........
      	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141)
      	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
      	at org.apache.catalina.authenticator.SingleSignOn.invoke(SingleSignOn.java:240)
      	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
      	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:502)
      	at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1132)
      	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:684)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1533)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1489)
      	- locked <0x00000006c844cea8> (a org.apache.tomcat.util.net.NioChannel)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
      	at java.lang.Thread.run(Thread.java:748)
      
      "Thread-1499929651452_###_Thread-1499929651449_###_http-nio-8443-exec-17" #371 daemon prio=5 os_prio=0 tid=0x00007f8305134000 nid=0x49be waiting for monitor entry [0x00007f7e9cac4000]
         java.lang.Thread.State: BLOCKED (on object monitor)
      	at ognl.OgnlRuntime.invokeMethod(OgnlRuntime.java:826)
      	- waiting to lock <0x00000006c690e5b8> (a java.lang.reflect.Method)
      	at ognl.OgnlRuntime.callAppropriateMethod(OgnlRuntime.java:1294)
      	at ognl.ObjectMethodAccessor.callMethod(ObjectMethodAccessor.java:68)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethodWithDebugInfo(XWorkMethodAccessor.java:117)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethod(XWorkMethodAccessor.java:108)
      	at ognl.OgnlRuntime.callMethod(OgnlRuntime.java:1370)
      	at ognl.ASTMethod.getValueBody(ASTMethod.java:91)
      	at ognl.SimpleNode.evaluateGetValueBody(SimpleNode.java:212)
      	at ognl.SimpleNode.getValue(SimpleNode.java:258)
      	at ognl.Ognl.getValue(Ognl.java:467)
      	at ognl.Ognl.getValue(Ognl.java:431)
      	at com.opensymphony.xwork2.ognl.OgnlUtil$3.execute(OgnlUtil.java:352)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.compileAndExecuteMethod(OgnlUtil.java:404)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.callMethod(OgnlUtil.java:350)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeAction(DefaultActionInvocation.java:430)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeActionOnly(DefaultActionInvocation.java:290)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:251)
      	........
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:245)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	.........
      	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141)
      	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
      	at org.apache.catalina.authenticator.SingleSignOn.invoke(SingleSignOn.java:240)
      	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
      	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:502)
      	at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1132)
      	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:684)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1533)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1489)
      	- locked <0x00000006c8605150> (a org.apache.tomcat.util.net.NioChannel)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
      	at java.lang.Thread.run(Thread.java:748)
      
      "Thread-1499929544435_###_Thread-1499929544432_###_http-nio-8443-exec-13" #367 daemon prio=5 os_prio=0 tid=0x00007f830512c800 nid=0x49ba runnable [0x00007f7e9cec6000]
         java.lang.Thread.State: RUNNABLE
      	at java.net.SocketInputStream.socketRead0(Native Method)
      	at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
      	at java.net.SocketInputStream.read(SocketInputStream.java:171)
      	at java.net.SocketInputStream.read(SocketInputStream.java:141)
      	at com.mysql.jdbc.util.ReadAheadInputStream.fill(ReadAheadInputStream.java:114)
      	at com.mysql.jdbc.util.ReadAheadInputStream.readFromUnderlyingStreamIfNecessary(ReadAheadInputStream.java:161)
      	at com.mysql.jdbc.util.ReadAheadInputStream.read(ReadAheadInputStream.java:189)
      	- locked <0x00000006c7201d08> (a com.mysql.jdbc.util.ReadAheadInputStream)
      	at com.mysql.jdbc.MysqlIO.readFully(MysqlIO.java:3014)
      	at com.mysql.jdbc.MysqlIO.reuseAndReadPacket(MysqlIO.java:3467)
      	at com.mysql.jdbc.MysqlIO.reuseAndReadPacket(MysqlIO.java:3456)
      	at com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:3997)
      	at com.mysql.jdbc.MysqlIO.sendCommand(MysqlIO.java:2468)
      	at com.mysql.jdbc.MysqlIO.sqlQueryDirect(MysqlIO.java:2629)
      	at com.mysql.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2713)
      	- locked <0x00000006c6a0cdd0> (a com.mysql.jdbc.JDBC4Connection)
      	at com.mysql.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2663)
      	at com.mysql.jdbc.StatementImpl.executeQuery(StatementImpl.java:1599)
      	- locked <0x00000006c6a0cdd0> (a com.mysql.jdbc.JDBC4Connection)
      	.........
      	at sun.reflect.GeneratedMethodAccessor44.invoke(Unknown Source)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	.........
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	..........
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	at ognl.OgnlRuntime.invokeMethod(OgnlRuntime.java:871)
      	- locked <0x00000006c690e5b8> (a java.lang.reflect.Method)
      	at ognl.OgnlRuntime.callAppropriateMethod(OgnlRuntime.java:1294)
      	at ognl.ObjectMethodAccessor.callMethod(ObjectMethodAccessor.java:68)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethodWithDebugInfo(XWorkMethodAccessor.java:117)
      	at com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor.callMethod(XWorkMethodAccessor.java:108)
      	at ognl.OgnlRuntime.callMethod(OgnlRuntime.java:1370)
      	at ognl.ASTMethod.getValueBody(ASTMethod.java:91)
      	at ognl.SimpleNode.evaluateGetValueBody(SimpleNode.java:212)
      	at ognl.SimpleNode.getValue(SimpleNode.java:258)
      	at ognl.Ognl.getValue(Ognl.java:467)
      	at ognl.Ognl.getValue(Ognl.java:431)
      	at com.opensymphony.xwork2.ognl.OgnlUtil$3.execute(OgnlUtil.java:352)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.compileAndExecuteMethod(OgnlUtil.java:404)
      	at com.opensymphony.xwork2.ognl.OgnlUtil.callMethod(OgnlUtil.java:350)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeAction(DefaultActionInvocation.java:430)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invokeActionOnly(DefaultActionInvocation.java:290)
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:251)
      	..........
      	at com.opensymphony.xwork2.DefaultActionInvocation.invoke(DefaultActionInvocation.java:245)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	...........
      	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141)
      	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
      	at org.apache.catalina.authenticator.SingleSignOn.invoke(SingleSignOn.java:240)
      	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
      	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:502)
      	at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1132)
      	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:684)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1533)
      	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1489)
      	- locked <0x00000006c52e67a8> (a org.apache.tomcat.util.net.NioChannel)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
      	at java.lang.Thread.run(Thread.java:748)
      
      

      Modified the syncInvoke checks to prevent this unnecessary blocking

       public static Object invokeMethod(Object target, Method method, Object[] argsArray)
          throws InvocationTargetException, IllegalAccessException
        {
          boolean syncInvoke = false;
          boolean checkPermission = false;
          boolean wasAccessible = true;
      
      
          synchronized (method) {
            if ((_methodAccessCache.get(method) == null))
            {
              if ((!Modifier.isPublic(method.getModifiers())) || (!Modifier.isPublic(method.getDeclaringClass().getModifiers())))
              {
                if (!(wasAccessible = method.isAccessible()))
                {
                   method.setAccessible(true);
                  _methodAccessCache.put(method, Boolean.TRUE);
                }
                else {
                  _methodAccessCache.put(method, Boolean.FALSE);
                }
              }
              else {
                _methodAccessCache.put(method, Boolean.FALSE);
              }
            }
            if (_methodAccessCache.get(method) == Boolean.TRUE))
            {
              syncInvoke = true;
            }
      
            if (((_securityManager != null) && (_methodPermCache.get(method) == null)) || (_methodPermCache.get(method) == Boolean.FALSE))
            {
              checkPermission = true;
            }
      
          }
      
          Object result;
          if (syncInvoke)
          {
            synchronized (method)
            {
              if (checkPermission)
              {
                try
                {
                  _securityManager.checkPermission(getPermission(method));
                  _methodPermCache.put(method, Boolean.TRUE);
                } catch (SecurityException ex) {
                  _methodPermCache.put(method, Boolean.FALSE);
                  throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
                }
              }
      
              Object result = method.invoke(target, argsArray);
      
              if (!wasAccessible)
              {
                method.setAccessible(false);
              }
            }
          }
          else {
            if (checkPermission)
            {
              try
              {
                _securityManager.checkPermission(getPermission(method));
                _methodPermCache.put(method, Boolean.TRUE);
              } catch (SecurityException ex) {
                _methodPermCache.put(method, Boolean.FALSE);
                throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
              }
            }
      
            result = method.invoke(target, argsArray);
          }
      
          return result;
        }
      
      
      1. ThreadBlock.war
        4.03 MB
        Santhana Preethi J
      2. threadDump.txt
        53 kB
        Santhana Preethi J

        Issue Links

          Activity

          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Can you post more informations about the multi-threaded scenario? How many concurrent requests? Single server or cluster? etc.

          This part of OGNL is very old (over 10 years now) and it wasn't changed till then. And nobody else reported such problem for 10 years which is rather unexpected.

          Show
          lukaszlenart Lukasz Lenart added a comment - Can you post more informations about the multi-threaded scenario ? How many concurrent requests? Single server or cluster? etc. This part of OGNL is very old (over 10 years now) and it wasn't changed till then. And nobody else reported such problem for 10 years which is rather unexpected.
          Hide
          rajacheran Raja added a comment -

          @Lukasz Lenart me too faced this issue. After starting an struts2 based application, if application-server receives multiple request for same url only one request will be processed and all other request will be blocked till first request complete its execution.

          In case if first thread hangs on I/O call or takes too much processing time then all other request will be blocked from entering action class method.

          Once first request processing is over every thing will starts to work fine.

          Show
          rajacheran Raja added a comment - @Lukasz Lenart me too faced this issue. After starting an struts2 based application, if application-server receives multiple request for same url only one request will be processed and all other request will be blocked till first request complete its execution. In case if first thread hangs on I/O call or takes too much processing time then all other request will be blocked from entering action class method. Once first request processing is over every thing will starts to work fine.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          But this this still doesn't give me enough information how to reproduce this issue. And from your description it looks like the issue is in I/O operation not the OGNL itself.

          Show
          lukaszlenart Lukasz Lenart added a comment - But this this still doesn't give me enough information how to reproduce this issue. And from your description it looks like the issue is in I/O operation not the OGNL itself.
          Hide
          Preethi Santhana Preethi J added a comment -

          Lukasz Lenart : To answer your question, I faced this issue when there were 10 concurrent requests to a single server.

          Also attached a simple application which can be used to reproduce the issue. From the attached threadDump, you can clearly see how two threads are blocked because the first thread is a long running one.

          Show
          Preethi Santhana Preethi J added a comment - Lukasz Lenart : To answer your question, I faced this issue when there were 10 concurrent requests to a single server. Also attached a simple application which can be used to reproduce the issue. From the attached threadDump, you can clearly see how two threads are blocked because the first thread is a long running one.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Thanks for the demo app! So it only happens on initial request, right?

          Show
          lukaszlenart Lukasz Lenart added a comment - Thanks for the demo app! So it only happens on initial request, right?
          Hide
          quaff zhouyanming added a comment -

          Is synchronization check necessary here? If necessary is it for all methods even public?

          Show
          quaff zhouyanming added a comment - Is synchronization check necessary here? If necessary is it for all methods even public?
          Hide
          Preethi Santhana Preethi J added a comment -

          Lukasz Lenart : Yes, in case of public methods it happens only for the first request. Suppose the first thread is long-running or gets hanged, the successive requests get blocked unnecessarily.

          Show
          Preethi Santhana Preethi J added a comment - Lukasz Lenart : Yes, in case of public methods it happens only for the first request. Suppose the first thread is long-running or gets hanged, the successive requests get blocked unnecessarily.
          Hide
          quaff zhouyanming added a comment -

          I think it's safe to add a short circuit.

          if (Modifier.isPublic(method.getModifiers()))
                  {
              			return method.invoke(target, argsArray);
                  }
          
          Show
          quaff zhouyanming added a comment - I think it's safe to add a short circuit. if (Modifier.isPublic(method.getModifiers())) { return method.invoke(target, argsArray); }
          Hide
          yasser.zamani Yasser Zamani added a comment -

          I am working on this. Strangely, `execute` method is locked some where else so `OgnlRuntime` blocks for it!

          Show
          yasser.zamani Yasser Zamani added a comment - I am working on this. Strangely, `execute` method is locked some where else so `OgnlRuntime` blocks for it!
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          This requires changes to OGNL itself, just keep in mind that this library is also used by other projects - Tiles, Thymleaf ...

          Show
          lukaszlenart Lukasz Lenart added a comment - This requires changes to OGNL itself, just keep in mind that this library is also used by other projects - Tiles, Thymleaf ...
          Hide
          yasser.zamani Yasser Zamani added a comment -

          Lukasz Lenart Tahnks. Yes I know but I found the solution and fortunately has no side effects at all. I'll present it tomorrow as a pull request where you will be able to see

          Show
          yasser.zamani Yasser Zamani added a comment - Lukasz Lenart Tahnks. Yes I know but I found the solution and fortunately has no side effects at all. I'll present it tomorrow as a pull request where you will be able to see
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Cool One more thing: the current 3.2.x OGNL branch is rather unstable (or not tested enough so you must target 3.0.x (tu support Struts 2.3.x) and 3.1.x (to support Struts 2.5.x)

          Show
          lukaszlenart Lukasz Lenart added a comment - Cool One more thing: the current 3.2.x OGNL branch is rather unstable (or not tested enough so you must target 3.0.x (tu support Struts 2.3.x) and 3.1.x (to support Struts 2.5.x)
          Hide
          yasser.zamani Yasser Zamani added a comment - - edited

          Thank you Lukasz Lenart, By the way I resolved the issue as below but now, upgrading S2 to use ognl 3.2.4 throws me a lot of failed tests and need a lot of changes to pass them.

          I could not find any 3.1.x branch on ognl's github to target? Do you mean I should create a new branch re-based on last 3.1.x series (e.g. ognl-3.1-support) and provide it as a pull request? then provide my solution on it as pull request again?

          my solution:

          Index: src/java/ognl/OgnlRuntime.java
          IDEA additional info:
          Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
          <+>UTF-8
          ===================================================================
          --- src/java/ognl/OgnlRuntime.java	(revision 92c2cd66f838a7bc8a110eff8752e4e96882935d)
          +++ src/java/ognl/OgnlRuntime.java	(revision )
          @@ -825,21 +825,48 @@
                   // only synchronize method invocation if it actually requires it
           
                   synchronized(method) {
          -            if (_methodAccessCache.get(method) == null
          -                || _methodAccessCache.get(method) == Boolean.TRUE) {
          +            if (_methodAccessCache.get(method) == null) {
          +                if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers()))
          +                {
          +                    if (!(((AccessibleObject) method).isAccessible()))
          +                    {
          +                        _methodAccessCache.put(method, Boolean.TRUE);
          +                    } else
          +                    {
          +                        _methodAccessCache.put(method, Boolean.FALSE);
          +                    }
          +                } else
          +                {
          +                    _methodAccessCache.put(method, Boolean.FALSE);
          +                }
          +            }
          +            if (_methodAccessCache.get(method) == Boolean.TRUE) {
                           syncInvoke = true;
                       }
           
          -            if (_securityManager != null && _methodPermCache.get(method) == null
          -                || _methodPermCache.get(method) == Boolean.FALSE) {
          +            if (_methodPermCache.get(method) == null) {
          +                if (_securityManager != null) {
          +                    try
          +                    {
          +                        _securityManager.checkPermission(getPermission(method));
          +                        _methodPermCache.put(method, Boolean.TRUE);
          +                    } catch (SecurityException ex) {
          +                        _methodPermCache.put(method, Boolean.FALSE);
          +                        throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
          +                    }
          +                }
          +                else {
          +                    _methodPermCache.put(method, Boolean.TRUE);
          +                }
          +            }
          +            if (_methodPermCache.get(method) == Boolean.FALSE) {
                           checkPermission = true;
                       }
                   }
           
                   Object result;
          -        boolean wasAccessible = true;
           
          -        if (syncInvoke)
          +        if (syncInvoke) //if is not public and is not accessible
                   {
                       synchronized(method)
                       {
          @@ -848,34 +875,14 @@
                               try
                               {
                                   _securityManager.checkPermission(getPermission(method));
          -                        _methodPermCache.put(method, Boolean.TRUE);
                               } catch (SecurityException ex) {
          -                        _methodPermCache.put(method, Boolean.FALSE);
                                   throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
                               }
                           }
           
          -                if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers()))
          -                {
          -                    if (!(wasAccessible = ((AccessibleObject) method).isAccessible()))
          -                    {
          -                        ((AccessibleObject) method).setAccessible(true);
          -                        _methodAccessCache.put(method, Boolean.TRUE);
          -                    } else
          -                    {
          -                        _methodAccessCache.put(method, Boolean.FALSE);
          -                    }
          -                } else
          -                {
          -                    _methodAccessCache.put(method, Boolean.FALSE);
          -                }
          -
          +                ((AccessibleObject) method).setAccessible(true);
                           result = method.invoke(target, argsArray);
          -
          -                if (!wasAccessible)
          -                {
          -                    ((AccessibleObject) method).setAccessible(false);
          -                }
          +                ((AccessibleObject) method).setAccessible(false);
                       }
                   } else
                   {
          @@ -884,9 +891,7 @@
                           try
                           {
                               _securityManager.checkPermission(getPermission(method));
          -                    _methodPermCache.put(method, Boolean.TRUE);
                           } catch (SecurityException ex) {
          -                    _methodPermCache.put(method, Boolean.FALSE);
                               throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
                           }
                       }
          
          Show
          yasser.zamani Yasser Zamani added a comment - - edited Thank you Lukasz Lenart , By the way I resolved the issue as below but now, upgrading S2 to use ognl 3.2.4 throws me a lot of failed tests and need a lot of changes to pass them. I could not find any 3.1.x branch on ognl's github to target? Do you mean I should create a new branch re-based on last 3.1.x series (e.g. ognl-3.1-support) and provide it as a pull request? then provide my solution on it as pull request again? my solution: Index: src/java/ognl/OgnlRuntime.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/java/ognl/OgnlRuntime.java (revision 92c2cd66f838a7bc8a110eff8752e4e96882935d) +++ src/java/ognl/OgnlRuntime.java (revision ) @@ -825,21 +825,48 @@ // only synchronize method invocation if it actually requires it synchronized (method) { - if (_methodAccessCache.get(method) == null - || _methodAccessCache.get(method) == Boolean .TRUE) { + if (_methodAccessCache.get(method) == null ) { + if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers())) + { + if (!(((AccessibleObject) method).isAccessible())) + { + _methodAccessCache.put(method, Boolean .TRUE); + } else + { + _methodAccessCache.put(method, Boolean .FALSE); + } + } else + { + _methodAccessCache.put(method, Boolean .FALSE); + } + } + if (_methodAccessCache.get(method) == Boolean .TRUE) { syncInvoke = true ; } - if (_securityManager != null && _methodPermCache.get(method) == null - || _methodPermCache.get(method) == Boolean .FALSE) { + if (_methodPermCache.get(method) == null ) { + if (_securityManager != null ) { + try + { + _securityManager.checkPermission(getPermission(method)); + _methodPermCache.put(method, Boolean .TRUE); + } catch (SecurityException ex) { + _methodPermCache.put(method, Boolean .FALSE); + throw new IllegalAccessException( "Method [" + method + "] cannot be accessed." ); + } + } + else { + _methodPermCache.put(method, Boolean .TRUE); + } + } + if (_methodPermCache.get(method) == Boolean .FALSE) { checkPermission = true ; } } Object result; - boolean wasAccessible = true ; - if (syncInvoke) + if (syncInvoke) // if is not public and is not accessible { synchronized (method) { @@ -848,34 +875,14 @@ try { _securityManager.checkPermission(getPermission(method)); - _methodPermCache.put(method, Boolean .TRUE); } catch (SecurityException ex) { - _methodPermCache.put(method, Boolean .FALSE); throw new IllegalAccessException( "Method [" + method + "] cannot be accessed." ); } } - if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers())) - { - if (!(wasAccessible = ((AccessibleObject) method).isAccessible())) - { - ((AccessibleObject) method).setAccessible( true ); - _methodAccessCache.put(method, Boolean .TRUE); - } else - { - _methodAccessCache.put(method, Boolean .FALSE); - } - } else - { - _methodAccessCache.put(method, Boolean .FALSE); - } - + ((AccessibleObject) method).setAccessible( true ); result = method.invoke(target, argsArray); - - if (!wasAccessible) - { - ((AccessibleObject) method).setAccessible( false ); - } + ((AccessibleObject) method).setAccessible( false ); } } else { @@ -884,9 +891,7 @@ try { _securityManager.checkPermission(getPermission(method)); - _methodPermCache.put(method, Boolean .TRUE); } catch (SecurityException ex) { - _methodPermCache.put(method, Boolean .FALSE); throw new IllegalAccessException( "Method [" + method + "] cannot be accessed." ); } }
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I have created two branches: ognl-3-1-x and ognl-3-0-x so you can open PRs against them

          Show
          lukaszlenart Lukasz Lenart added a comment - I have created two branches: ognl-3-1-x and ognl-3-0-x so you can open PRs against them
          Hide
          yasser.zamani Yasser Zamani added a comment -

          Lukasz Lenart, thanks a lot! I have created first one against ognl 3.1.x branch. Struts 2.5.13-SNAPSHOT successfully passes all test against it. I also confirmed that it also fixes this issue

          As support of Struts 2.3.x is only for security issues (right?), do we need do same as above for that?

          Show
          yasser.zamani Yasser Zamani added a comment - Lukasz Lenart , thanks a lot! I have created first one against ognl 3.1.x branch. Struts 2.5.13-SNAPSHOT successfully passes all test against it. I also confirmed that it also fixes this issue As support of Struts 2.3.x is only for security issues (right?), do we need do same as above for that?

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              Preethi Santhana Preethi J
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development