Uploaded image for project: 'Commons Digester'
  1. Commons Digester
  2. DIGESTER-161

Document thread-safety in javadoc of Rule class

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Resolved
    • Trivial
    • Resolution: Fixed
    • 3.1
    • None

    Description

      I discovered a problem today with some code that was reusing a custom Rule in multiple threads, even though each thread was creating its own digester. It seems that Digester.addRule is calling rule.setDigester and if the rule is shared across multiple threads, the calls to begin/end can get tangled across threads.

      It is obvious that Rules are not meant to be shared, but the javadoc <http://commons.apache.org/digester/apidocs/org/apache/commons/digester3/Rule.html> seems to be implying the opposite and is confusing at best. It talks about the rules being stateless, even though the framework itself is changing its state with rule.setDigester(digester). It further states that since all state is part of the digester, the rule is safe under all cases, which is very misleading.

      " ... Rule objects should be stateless, ie they should not update any instance member during the parsing process. A rule instance that changes state will encounter problems if invoked in a "nested" manner; this can happen if the same instance is added to digester multiple times or if a wildcard pattern is used which can match both an element and a child of the same element. The digester object stack and named stacks should be used to store any state that a rule requires, making the rule class safe under all possible uses. ..."

      I think the statement above should be reworded to be more correct and avoid confusion. Down the line, maybe the digester accessed by the rule should be a ThreadLocal.

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            simone.tripodi Simone Tripodi
            epapa Eduard Papa
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified

                Slack

                  Issue deployment