[xwiki-devs] Potential synchronization issues
Hi, I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues. For example take DefaultObservationManager: private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>(); It has for ex a addListener() method. Imagine several threads all calling addListener(). Since HashMap is not synchronized this can cause problems. Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex). WDYT? Do you agree there's a potential bug in the case above? Thanks -Vincent
Vincent Massol wrote:
Hi,
I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues.
For example take DefaultObservationManager:
private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>();
It has for ex a addListener() method.
Imagine several threads all calling addListener().
Since HashMap is not synchronized this can cause problems.
Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex).
WDYT? Do you agree there's a potential bug in the case above?
+1
Thanks -Vincent _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
+1 On Tue, Sep 30, 2008 at 5:28 PM, Vincent Massol <[email protected]> wrote:
Hi,
I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues.
For example take DefaultObservationManager:
private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>();
It has for ex a addListener() method.
Imagine several threads all calling addListener().
Since HashMap is not synchronized this can cause problems.
Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex).
WDYT? Do you agree there's a potential bug in the case above?
Thanks -Vincent _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
+1 Vincent Massol wrote:
Hi,
I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues.
For example take DefaultObservationManager:
private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>();
It has for ex a addListener() method.
Imagine several threads all calling addListener().
Since HashMap is not synchronized this can cause problems.
Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex).
WDYT? Do you agree there's a potential bug in the case above?
Thanks -Vincent _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
Hi, Vincent Massol napsal(a):
Hi,
I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues.
For example take DefaultObservationManager:
private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>();
It has for ex a addListener() method.
Imagine several threads all calling addListener().
Since HashMap is not synchronized this can cause problems.
Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex).
I prefer more general term "thread safe" then restrictive "synchronized". Some structs are thread safe without synchronization/locking (partly ConcurrentHashMap from JSE 5.0). Zdenek Machac
WDYT? Do you agree there's a potential bug in the case above?
Thanks -Vincent _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On Oct 1, 2008, at 11:32 AM, Zdenek Machac wrote:
Hi,
Vincent Massol napsal(a):
Hi,
I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues.
For example take DefaultObservationManager:
private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>();
It has for ex a addListener() method.
Imagine several threads all calling addListener().
Since HashMap is not synchronized this can cause problems.
Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex).
I prefer more general term "thread safe" then restrictive "synchronized". Some structs are thread safe without synchronization/locking (partly ConcurrentHashMap from JSE 5.0).
yes you're right that's what I meant: "thread safety". Thanks -Vincent
WDYT? Do you agree there's a potential bug in the case above?
Thanks -Vincent
ok, done for DefaultHTMLCleaner and DefaultObservationManager. Please all be aware of this and verify the component is thread safe when you work on one. Thanks to Zdenek for suggesting the usage of ConcurrentHashMap. Thanks -Vincent On Oct 1, 2008, at 11:55 AM, Vincent Massol wrote:
On Oct 1, 2008, at 11:32 AM, Zdenek Machac wrote:
Hi,
Vincent Massol napsal(a):
Hi,
I think we might need to review our singleton components (i.e. all of them ;)) for sync. issues.
For example take DefaultObservationManager:
private Map<String, List<RegisteredListener>> listeners = new HashMap<String, List<RegisteredListener>>();
It has for ex a addListener() method.
Imagine several threads all calling addListener().
Since HashMap is not synchronized this can cause problems.
Thus shared objects should all be synchronized or they should only be filled once (as in an initialize method for ex).
I prefer more general term "thread safe" then restrictive "synchronized". Some structs are thread safe without synchronization/locking (partly ConcurrentHashMap from JSE 5.0).
yes you're right that's what I meant: "thread safety".
Thanks -Vincent
WDYT? Do you agree there's a potential bug in the case above?
Thanks -Vincent
participants (5)
-
Anca Paula Luca -
Marius Dumitru Florea -
Thomas Mortagne -
Vincent Massol -
Zdenek Machac