Claudenw commented on PR #704:
URL: https://github.com/apache/creadur-rat/pull/704#issuecomment-5100970228

   @gnodet , again thank you for your work on parallelizing RAT.
   
   I think that the SPDX Matcher needs attention to make this functionally 
complete.
   
   **Issue**:
   Currently if two threads are running and each detect different SPDX ids they 
will both report that they have seen both Ids.
   
   
   **Analysis**:
   The SPDX matcher factory is created as a static INSTANCE in the 
SPDXMatcherFactory class.  The factory has a `lastMatch` set variable that 
tracks the SPDX Ids that were seen in the scan of the last file.
   
   The SPDXMatcherFactory creates instances of the SPDXMatcher class.  Each of 
those instances point contain a reference to the factory.  When they are 
triggered they insert their ID into the `lastMatch` set.
   
   When the matcher is executed it calls the `check` method in the 
SPDXMatcherFactory.  the SPDXMatcherFactory check verifies that the document 
has not be scanned and then scans for _ALL_ defined matchers and places them 
into the `lastMatch`.  If the SPDXMatcherFactory has already checked no action 
is taken.
   
   When the matcher is queried to see if it has found a match it checks to see 
if its ID is in the `SPDXMatcherFactory.lastMach` .
   
   There is a MATCHER_MAP static var in the SPDXMatcherFactory that could 
probably become a local variable.
   
   The data that the Matcher is updating is contained in the SPDXMatcherFactory 
(probably a bad name at this point).
   
   The SPDXBuilder is called when the configuration specifies an SPDX matcher 
is needed.  The Builder calls to the SPDXMatcherFactory to create the matcher.
   
   
   **Potential Solution**:
   
   You have made the creation of the configuration synchronized.  During the 
creation of the configuration we use the factory instance to create SPDX 
matchers.  
   
   If the SPDXMatcherFactory were made thread local to the SPDXBuilder, and the 
static variables in the SPDXMatcherFactory we mitigated I think this would 
solve the problem.
   
   **Question**:
   Are you willing to take on this change as part of your PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to