ICEfaces
  1. ICEfaces
  2. ICE-4539

UploadServer can fail with frameworks that rely on ThreadLocals

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.1
    • Fix Version/s: 1.8.2-RC1, 1.8.2
    • Component/s: Framework, ICE-Components
    • Labels:
      None
    • Environment:
      spring file upload

      Description

      The UploadServer has logic where it uses a separate, non-JSF thread, to load the file content. We bypass this logic when another framework (e.g Seam, Spring Web Flow) is relying on ThreadLocals to do what they need to do. The check to ensure that the appropriate code path is selected is not very generic. It currently only supports Seam and Spring Web Flow. The forum user was using Spring Security and ran into problems using the file upload component. They were able to work around it by modifying the source code to also check for Spring Security. However, this strategy is a bit of a treadmill in that we'll always potentially be behind in adding checks for the next framework.

        Activity

        Deryk Sinotte created issue -
        Hide
        Mark Collette added a comment -

        The UploadServer detects typical situations where we know there are ThreadLocal fields, like with Seam and Spring, and uses the current thread to do the lifecycle, otherwise it will default to using a different thread to do the lifecycle, leaving the current thread to continue processing uploaded data. This is to avoid losing performance unnecessarily. This specific problem is that we're not using the current thread in a Spring Security environment (I guess they're not using anything else from Spring), which also uses some ThreadLocal field(s). The generic problem is that we'll never be able to anticipate third party library uses of ThreadLocal fields.

        From talking with Deryk, it looks like the most straightforward solution is to add two more criteria to the decision to use the current thread for the lifecycle. The first being a check for Spring Security, and the second being a flag to force using the conservative threading model. That way, developers can just set a context-param, and not have to change or recompile ICEfaces, when integrating with any other libraries. At least as it pertains to uploading files.

        Show
        Mark Collette added a comment - The UploadServer detects typical situations where we know there are ThreadLocal fields, like with Seam and Spring, and uses the current thread to do the lifecycle, otherwise it will default to using a different thread to do the lifecycle, leaving the current thread to continue processing uploaded data. This is to avoid losing performance unnecessarily. This specific problem is that we're not using the current thread in a Spring Security environment (I guess they're not using anything else from Spring), which also uses some ThreadLocal field(s). The generic problem is that we'll never be able to anticipate third party library uses of ThreadLocal fields. From talking with Deryk, it looks like the most straightforward solution is to add two more criteria to the decision to use the current thread for the lifecycle. The first being a check for Spring Security, and the second being a flag to force using the conservative threading model. That way, developers can just set a context-param, and not have to change or recompile ICEfaces, when integrating with any other libraries. At least as it pertains to uploading files.
        Hide
        Adrian Gygax added a comment -

        +1 for this solution

        Show
        Adrian Gygax added a comment - +1 for this solution
        Ken Fyten made changes -
        Field Original Value New Value
        Salesforce Case []
        Fix Version/s 1.8.2 [ 10190 ]
        Affects [Compatibility/Configuration]
        Assignee Priority P2
        Assignee Greg Dick [ greg.dick ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #18988 Wed Jun 24 09:24:39 MDT 2009 greg.dick ICE-4539 Added check for Spring security and a context parameter when deciding to execute JSF lifecycle in calling thread
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/core/src/com/icesoft/faces/webapp/http/core/UploadServer.java
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #18989 Wed Jun 24 09:25:57 MDT 2009 greg.dick ICE-4539 Added check for Spring security context
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/core/src/com/icesoft/util/SeamUtilities.java
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #18992 Wed Jun 24 12:49:29 MDT 2009 greg.dick ICE-4539 Added 'force' to property name to indicate how it should be used.
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/core/src/com/icesoft/faces/webapp/http/core/UploadServer.java
        Hide
        Greg Dick added a comment -

        I've put the code in pretty much as posted by the forum user with the addition of a context-parameter that forces the code to use the safe (but slow) practice of executing the lifecycle on the calling thread.

        The context parameter name is com.icesoft.faces.forceLifecycleOnCallingThread and the default value is false.

        Show
        Greg Dick added a comment - I've put the code in pretty much as posted by the forum user with the addition of a context-parameter that forces the code to use the safe (but slow) practice of executing the lifecycle on the calling thread. The context parameter name is com.icesoft.faces.forceLifecycleOnCallingThread and the default value is false.
        Greg Dick made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Ken Fyten made changes -
        Fix Version/s 1.8.2-RC1 [ 10210 ]
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Assignee Priority P2

          People

          • Assignee:
            Greg Dick
            Reporter:
            Deryk Sinotte
          • Votes:
            3 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: