You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Chris DiGiano (JIRA)" <ji...@apache.org> on 2006/05/22 15:12:29 UTC

[jira] Created: (VFS-50) Threading issue with AbstractFileSystem.fireEvent

Threading issue with AbstractFileSystem.fireEvent
-------------------------------------------------

         Key: VFS-50
         URL: http://issues.apache.org/jira/browse/VFS-50
     Project: Commons VFS
        Type: Bug

    Reporter: Chris DiGiano
    Priority: Minor


I make heavy use of DelegateFileObject in my code and recently ran into a threading issue when a change in one file was trying to automatically update dependent DelegateFileObjects. While events where still being propagated, one DelegateFileObject removed itself as an event listener. This caused an index-of-range problem in AbstractFileSystem.fireEvent because the listener list had shrunk, but the "count" had already been fixed.

It appears that the current implementation of fireEvent is not entirely thread safe, if event handlers can still change the listener list in place. I will attach a patch that works for me.


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Closed: (VFS-50) Threading issue with AbstractFileSystem.fireEvent

Posted by "Mario Ivankovits (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/VFS-50?page=all ]
     
Mario Ivankovits closed VFS-50:
-------------------------------


> Threading issue with AbstractFileSystem.fireEvent
> -------------------------------------------------
>
>          Key: VFS-50
>          URL: http://issues.apache.org/jira/browse/VFS-50
>      Project: Commons VFS
>         Type: Bug

>     Reporter: Chris DiGiano
>     Assignee: Mario Ivankovits
>     Priority: Minor
>      Fix For: Nightly Builds

>
> I make heavy use of DelegateFileObject in my code and recently ran into a threading issue when a change in one file was trying to automatically update dependent DelegateFileObjects. While events where still being propagated, one DelegateFileObject removed itself as an event listener. This caused an index-of-range problem in AbstractFileSystem.fireEvent because the listener list had shrunk, but the "count" had already been fixed.
> It appears that the current implementation of fireEvent is not entirely thread safe, if event handlers can still change the listener list in place. I will attach a patch that works for me.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Commented: (VFS-50) Threading issue with AbstractFileSystem.fireEvent

Posted by "Chris DiGiano (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/VFS-50?page=comments#action_12414099 ] 

Chris DiGiano commented on VFS-50:
----------------------------------

I've confirmed that Mario's slightly modified fix addresses the problem I was noticing in my own project. Thanks Mario!

> Threading issue with AbstractFileSystem.fireEvent
> -------------------------------------------------
>
>          Key: VFS-50
>          URL: http://issues.apache.org/jira/browse/VFS-50
>      Project: Commons VFS
>         Type: Bug

>     Reporter: Chris DiGiano
>     Assignee: Mario Ivankovits
>     Priority: Minor
>      Fix For: Nightly Builds

>
> I make heavy use of DelegateFileObject in my code and recently ran into a threading issue when a change in one file was trying to automatically update dependent DelegateFileObjects. While events where still being propagated, one DelegateFileObject removed itself as an event listener. This caused an index-of-range problem in AbstractFileSystem.fireEvent because the listener list had shrunk, but the "count" had already been fixed.
> It appears that the current implementation of fireEvent is not entirely thread safe, if event handlers can still change the listener list in place. I will attach a patch that works for me.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Resolved: (VFS-50) Threading issue with AbstractFileSystem.fireEvent

Posted by "Mario Ivankovits (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/VFS-50?page=all ]
     
Mario Ivankovits resolved VFS-50:
---------------------------------

    Fix Version: Nightly Builds
     Resolution: Fixed
      Assign To: Mario Ivankovits

Well, this wasnt really a "threading issue" but a normal bug.
Since every access to the "listeners" is synchronized on the "listenerMap" both are save. There is no access to the "listeners" outside an "listenerMap" synchronization.

I slightly modified your patch.

Thanks for reporting it.

> Threading issue with AbstractFileSystem.fireEvent
> -------------------------------------------------
>
>          Key: VFS-50
>          URL: http://issues.apache.org/jira/browse/VFS-50
>      Project: Commons VFS
>         Type: Bug

>     Reporter: Chris DiGiano
>     Assignee: Mario Ivankovits
>     Priority: Minor
>      Fix For: Nightly Builds

>
> I make heavy use of DelegateFileObject in my code and recently ran into a threading issue when a change in one file was trying to automatically update dependent DelegateFileObjects. While events where still being propagated, one DelegateFileObject removed itself as an event listener. This caused an index-of-range problem in AbstractFileSystem.fireEvent because the listener list had shrunk, but the "count" had already been fixed.
> It appears that the current implementation of fireEvent is not entirely thread safe, if event handlers can still change the listener list in place. I will attach a patch that works for me.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Commented: (VFS-50) Threading issue with AbstractFileSystem.fireEvent

Posted by "Chris DiGiano (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/VFS-50?page=comments#action_12412771 ] 

Chris DiGiano commented on VFS-50:
----------------------------------

By the way, it's not clear to me that the existing synchronization on listenerMap provides any real threading protection. The references itself is unlikely to change. Instead it's the contents of the map that we need to worry about.

> Threading issue with AbstractFileSystem.fireEvent
> -------------------------------------------------
>
>          Key: VFS-50
>          URL: http://issues.apache.org/jira/browse/VFS-50
>      Project: Commons VFS
>         Type: Bug

>     Reporter: Chris DiGiano
>     Priority: Minor

>
> I make heavy use of DelegateFileObject in my code and recently ran into a threading issue when a change in one file was trying to automatically update dependent DelegateFileObjects. While events where still being propagated, one DelegateFileObject removed itself as an event listener. This caused an index-of-range problem in AbstractFileSystem.fireEvent because the listener list had shrunk, but the "count" had already been fixed.
> It appears that the current implementation of fireEvent is not entirely thread safe, if event handlers can still change the listener list in place. I will attach a patch that works for me.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[jira] Commented: (VFS-50) Threading issue with AbstractFileSystem.fireEvent

Posted by "Chris DiGiano (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/VFS-50?page=comments#action_12412770 ] 

Chris DiGiano commented on VFS-50:
----------------------------------

Here's a patch that makes a copy of the listener list to avoid race conditions:

Index: src/java/org/apache/commons/vfs/provider/AbstractFileSystem.java
===================================================================
--- src/java/org/apache/commons/vfs/provider/AbstractFileSystem.java    (revision 408551)
+++ src/java/org/apache/commons/vfs/provider/AbstractFileSystem.java    (working copy)
@@ -484,9 +484,12 @@
         synchronized (listenerMap)
         {
             final FileObject file = event.getFile();
-            final ArrayList listeners = (ArrayList) listenerMap.get(file.getName());
-            if (listeners != null)
+//            final ArrayList listeners = (ArrayList) listenerMap.get(file.getName());
+//            if (listeners != null)
+            if (listenerMap.containsKey(file.getName()))
             {
+                // Make a copy of the listeners to avoid race conditions. (Chris DiGiano)
+                final ArrayList listeners = new ArrayList((ArrayList) listenerMap.get(file.getName()));
                 final int count = listeners.size();
                 for (int i = 0; i < count; i++)
                 {


> Threading issue with AbstractFileSystem.fireEvent
> -------------------------------------------------
>
>          Key: VFS-50
>          URL: http://issues.apache.org/jira/browse/VFS-50
>      Project: Commons VFS
>         Type: Bug

>     Reporter: Chris DiGiano
>     Priority: Minor

>
> I make heavy use of DelegateFileObject in my code and recently ran into a threading issue when a change in one file was trying to automatically update dependent DelegateFileObjects. While events where still being propagated, one DelegateFileObject removed itself as an event listener. This caused an index-of-range problem in AbstractFileSystem.fireEvent because the listener list had shrunk, but the "count" had already been fixed.
> It appears that the current implementation of fireEvent is not entirely thread safe, if event handlers can still change the listener list in place. I will attach a patch that works for me.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org