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

        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
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: