You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avalon.apache.org by Marcus Crafter <cr...@fztig938.bank.dresdner.net> on 2001/12/18 17:04:28 UTC

Parameters & thread safety

Hi All,

	This time with a subject line!.. :-)

	Cheers,

	Marcus

On Tue, Dec 18, 2001 at 05:03:01PM +0100, crafterm wrote:
> Hi All,
> 
> 	Hope all is well!
> 
> 	I've just been doing some debugging with our application here in
> 	Dresdner Bank, which has led me to have a closer look at the
> 	Parameters class. From my analysis the Parameters class is not
> 	thread safe.
> 
> 	(Parameters uses HashMap as it's internal store, and does not
> 	synchronize any of its mutator methods).
> 	
> 	I'm just wondering if this is intended (ie. like HashMap in the
> 	Collections API - up to the developer to synchronize access) or
> 	whether this is should be fixed as it can currently cause grief
> 	in Cocoon under load.
> 
> 	Any thoughts/comments/etc ?
> 
> 	Cheers,
> 
> 	Marcus
> 
> -- 
>         .....
>      ,,$$$$$$$$$,      Marcus Crafter
>     ;$'      '$$$$:    Computer Systems Engineer
>     $:         $$$$:   ManageSoft GmbH
>      $       o_)$$$:   82-84 Mainzer Landstrasse
>      ;$,    _/\ &&:'   60327 Frankfurt Germany
>        '     /( &&&
>            \_&&&&'     Email : Marcus.Crafter@managesoft.com
>           &&&&.        Business Hours : +49 69 9757 200
>     &&&&&&&:

-- 
        .....
     ,,$$$$$$$$$,      Marcus Crafter
    ;$'      '$$$$:    Computer Systems Engineer
    $:         $$$$:   ManageSoft GmbH
     $       o_)$$$:   82-84 Mainzer Landstrasse
     ;$,    _/\ &&:'   60327 Frankfurt Germany
       '     /( &&&
           \_&&&&'     Email : Marcus.Crafter@managesoft.com
          &&&&.        Business Hours : +49 69 9757 200
    &&&&&&&:

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Marcus Crafter <cr...@fztig938.bank.dresdner.net>.
Hi Berin,

On Tue, Dec 18, 2001 at 02:23:24PM -0500, Berin Loritsch wrote:
> >  components */
> >  private Parameters emptyParam = new Parameters();
> >
> >  /** HashMap relating labels to view names */
> >  private HashMap view_label_map = new HashMap(1);
> >
> >	The 'emptyParam' is passed into several cocoon components. If a
> >	developer sets a parameter in a selector for example, then 2
> >	threads can overwrite each other with different results (the current
> >	problem we have here).
> 
> Is emptyParam supposed to be a read-only object, or something that is 
> started at the beginning of a pipeline?

	I'm not sure if it's mean't to be read-only, as that code has been
	around for a long time, and I'm not sure what the initial intentions
	were by the developer that wrote it.

	I think it would be more useful if it was started at the
	beginning of a pipeline. Then it's at least separate from all other
	pipelines, and threads.

	What do you think ?

> >	If the Parameters object remains thread unsafe, perhaps it should be
> >	documented in it's class level javadocs like the Collection
> >	classes so it's obvious to the developer (I'll submit a patch if
> >	you like ?).
> 
> Sounds good.

	Attached. Hope it's ok.

	Cheers,
	
	Marcus

-- 
        .....
     ,,$$$$$$$$$,      Marcus Crafter
    ;$'      '$$$$:    Computer Systems Engineer
    $:         $$$$:   ManageSoft GmbH
     $       o_)$$$:   82-84 Mainzer Landstrasse
     ;$,    _/\ &&:'   60327 Frankfurt Germany
       '     /( &&&
           \_&&&&'     Email : Marcus.Crafter@managesoft.com
          &&&&.        Business Hours : +49 69 9757 200
    &&&&&&&:

Re: Parameters & thread safety

Posted by Berin Loritsch <bl...@apache.org>.
Marcus Crafter wrote:

> Hi Berin,
> 
> On Tue, Dec 18, 2001 at 11:20:18AM -0500, Berin Loritsch wrote:
> 
>>In what way are the same set of parameters used by different threads in 
>>Cocoon?
>>
>>We never addressed ThreadSafety as one of the Contracts for either 
>>Parameters
>>or Configuration.
>>
> 
> 	Have a look at this snippet from a generated sitemap:
> 
> ----------------------
> public class sitemap_xmap extends AbstractSitemap {
>   static final String LOCATION = "org.apache.cocoon.www.sitemap_xmap";
> 
>   static {
>     dateCreated = 1007984808622L;
>   }
> 
>   /** An empty <code>Parameter</code> used to pass to the sitemap components */
>   private Parameters emptyParam = new Parameters();
> 
>   /** HashMap relating labels to view names */
>   private HashMap view_label_map = new HashMap(1);
> 
> <snip>...
> 
>   if (isSelected("userstatus", "permitted", listOfMaps,
>                emptyParam, objectModel)) {
> ----------------------
> 
> 	The 'emptyParam' is passed into several cocoon components. If a
> 	developer sets a parameter in a selector for example, then 2
> 	threads can overwrite each other with different results (the current
> 	problem we have here).


Is emptyParam supposed to be a read-only object, or something that is started
at the beginning of a pipeline?


> 
> 	In Cocoon I think the problem is more fundamental in that the
> 	parameters object is actually an instance level reference in the
> 	sitemap object (I'm not sure if that was intended ? ie. to share
> 	parameters between pipelines), but I think even a thread safe
> 	parameters object would not fix thread safety there - there
> 	needs to be a local Parameters object per thread passing through
> 	the sitemap, or a read-only Parameters object if the developer
> 	shouldn't be doing this at all.


Yep, this can only be fixed with ThreadLocal parameters objects.


> 
> 	If the Parameters object remains thread unsafe, perhaps it should be
> 	documented in it's class level javadocs like the Collection
> 	classes so it's obvious to the developer (I'll submit a patch if
> 	you like ?).


Sounds good.




-- 

"They that give up essential liberty to obtain a little temporary safety
  deserve neither liberty nor safety."
                 - Benjamin Franklin


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Berin Loritsch <bl...@apache.org>.
Peter Donald wrote:

> On Wed, 19 Dec 2001 08:28, giacomo wrote:
> 
>>I see what you mean. I've obviously never really looked at the
>>Parameters interface but now I've realized that there isn't similarity
>>with the Configuration class which *is* an interface whereas
>>Parameters is *not*. I don't know the intention why Parameters are
>>not designed like Configuration.
>>
>>Any comments from core Avalonians why Parameters isn't an interface?
>>
> 
> It was originally a class when it was inherited from stylebook or wherever it 
> came from. No one could come up with a legitimate reason for changing it so 
> we didn't ;)


It came from Cocoon!




----------------------------------------------------
Sign Up for NetZero Platinum Today
Only $9.95 per month!
http://my.netzero.net/s/signup?r=platinum&refcd=PT97

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Peter Donald <pe...@apache.org>.
On Wed, 19 Dec 2001 08:28, giacomo wrote:
> I see what you mean. I've obviously never really looked at the
> Parameters interface but now I've realized that there isn't similarity
> with the Configuration class which *is* an interface whereas
> Parameters is *not*. I don't know the intention why Parameters are
> not designed like Configuration.
>
> Any comments from core Avalonians why Parameters isn't an interface?

It was originally a class when it was inherited from stylebook or wherever it 
came from. No one could come up with a legitimate reason for changing it so 
we didn't ;)

-- 
Cheers,

Pete

---------------------------------------------------
"Therefore it can be said that victorious warriors 
win first, and then go to battle, while defeated 
warriors go to battle first, and then seek to win." 
              - Sun Tzu, the Art Of War
---------------------------------------------------

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by giacomo <gi...@apache.org>.
On Tue, 18 Dec 2001, Marcus Crafter wrote:

> Hi Berin,
>
> On Tue, Dec 18, 2001 at 11:20:18AM -0500, Berin Loritsch wrote:
> >
> > In what way are the same set of parameters used by different threads in
> > Cocoon?
> >
> > We never addressed ThreadSafety as one of the Contracts for either
> > Parameters
> > or Configuration.
>
> 	Have a look at this snippet from a generated sitemap:
>
> ----------------------
> public class sitemap_xmap extends AbstractSitemap {
>   static final String LOCATION = "org.apache.cocoon.www.sitemap_xmap";
>
>   static {
>     dateCreated = 1007984808622L;
>   }
>
>   /** An empty <code>Parameter</code> used to pass to the sitemap components */
>   private Parameters emptyParam = new Parameters();
>
>   /** HashMap relating labels to view names */
>   private HashMap view_label_map = new HashMap(1);
>
> <snip>...
>
>   if (isSelected("userstatus", "permitted", listOfMaps,
>                emptyParam, objectModel)) {
> ----------------------
>
> 	The 'emptyParam' is passed into several cocoon components. If a
> 	developer sets a parameter in a selector for example, then 2
> 	threads can overwrite each other with different results (the current
> 	problem we have here).
>
> 	In Cocoon I think the problem is more fundamental in that the
> 	parameters object is actually an instance level reference in the
> 	sitemap object (I'm not sure if that was intended ? ie. to share
> 	parameters between pipelines), but I think even a thread safe
> 	parameters object would not fix thread safety there - there
> 	needs to be a local Parameters object per thread passing through
> 	the sitemap, or a read-only Parameters object if the developer
> 	shouldn't be doing this at all.

I see what you mean. I've obviously never really looked at the
Parameters interface but now I've realized that there isn't similarity
with the Configuration class which *is* an interface whereas
Parameters is *not*. I don't know the intention why Parameters are
not designed like Configuration.

Any comments from core Avalonians why Parameters isn't an interface?

Giacomo

>
> 	If the Parameters object remains thread unsafe, perhaps it should be
> 	documented in it's class level javadocs like the Collection
> 	classes so it's obvious to the developer (I'll submit a patch if
> 	you like ?).
>
> 	Cheers,
>
> 	Marcus
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Marcus Crafter <cr...@fztig938.bank.dresdner.net>.
Hi Berin,

On Tue, Dec 18, 2001 at 11:20:18AM -0500, Berin Loritsch wrote:
> 
> In what way are the same set of parameters used by different threads in 
> Cocoon?
> 
> We never addressed ThreadSafety as one of the Contracts for either 
> Parameters
> or Configuration.

	Have a look at this snippet from a generated sitemap:

----------------------
public class sitemap_xmap extends AbstractSitemap {
  static final String LOCATION = "org.apache.cocoon.www.sitemap_xmap";

  static {
    dateCreated = 1007984808622L;
  }

  /** An empty <code>Parameter</code> used to pass to the sitemap components */
  private Parameters emptyParam = new Parameters();

  /** HashMap relating labels to view names */
  private HashMap view_label_map = new HashMap(1);

<snip>...

  if (isSelected("userstatus", "permitted", listOfMaps,
               emptyParam, objectModel)) {
----------------------

	The 'emptyParam' is passed into several cocoon components. If a
	developer sets a parameter in a selector for example, then 2
	threads can overwrite each other with different results (the current
	problem we have here).

	In Cocoon I think the problem is more fundamental in that the
	parameters object is actually an instance level reference in the
	sitemap object (I'm not sure if that was intended ? ie. to share
	parameters between pipelines), but I think even a thread safe
	parameters object would not fix thread safety there - there
	needs to be a local Parameters object per thread passing through
	the sitemap, or a read-only Parameters object if the developer
	shouldn't be doing this at all.

	If the Parameters object remains thread unsafe, perhaps it should be
	documented in it's class level javadocs like the Collection
	classes so it's obvious to the developer (I'll submit a patch if
	you like ?).

	Cheers,

	Marcus

-- 
        .....
     ,,$$$$$$$$$,      Marcus Crafter
    ;$'      '$$$$:    Computer Systems Engineer
    $:         $$$$:   ManageSoft GmbH
     $       o_)$$$:   82-84 Mainzer Landstrasse
     ;$,    _/\ &&:'   60327 Frankfurt Germany
       '     /( &&&
           \_&&&&'     Email : Marcus.Crafter@managesoft.com
          &&&&.        Business Hours : +49 69 9757 200
    &&&&&&&:

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Berin Loritsch <bl...@apache.org>.
Marcus Crafter wrote:

> Hi All,
> 
> 	This time with a subject line!.. :-)
> 
> 	Cheers,
> 
> 	Marcus


In what way are the same set of parameters used by different threads in Cocoon?

We never addressed ThreadSafety as one of the Contracts for either Parameters
or Configuration.


> 
> On Tue, Dec 18, 2001 at 05:03:01PM +0100, crafterm wrote:
> 
>>Hi All,
>>
>>	Hope all is well!
>>
>>	I've just been doing some debugging with our application here in
>>	Dresdner Bank, which has led me to have a closer look at the
>>	Parameters class. From my analysis the Parameters class is not
>>	thread safe.
>>
>>	(Parameters uses HashMap as it's internal store, and does not
>>	synchronize any of its mutator methods).
>>	
>>	I'm just wondering if this is intended (ie. like HashMap in the
>>	Collections API - up to the developer to synchronize access) or
>>	whether this is should be fixed as it can currently cause grief
>>	in Cocoon under load.
>>
>>	Any thoughts/comments/etc ?
>>
>>	Cheers,
>>
>>	Marcus
>>
>>-- 
>>        .....
>>     ,,$$$$$$$$$,      Marcus Crafter
>>    ;$'      '$$$$:    Computer Systems Engineer
>>    $:         $$$$:   ManageSoft GmbH
>>     $       o_)$$$:   82-84 Mainzer Landstrasse
>>     ;$,    _/\ &&:'   60327 Frankfurt Germany
>>       '     /( &&&
>>           \_&&&&'     Email : Marcus.Crafter@managesoft.com
>>          &&&&.        Business Hours : +49 69 9757 200
>>    &&&&&&&:
>>
> 



-- 

"They that give up essential liberty to obtain a little temporary safety
  deserve neither liberty nor safety."
                 - Benjamin Franklin


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Berin Loritsch <bl...@apache.org>.
Klaus Sonnenleiter wrote:

> I think it should be up to the developer to synchronize when necessary. 
> Otherwise you'll end up with lots of overhead for synchronizing puts 
> where there is possibly no need at all.


We _can_ provide a SynchronizedParameters object that wraps the other.




-- 

"They that give up essential liberty to obtain a little temporary safety
  deserve neither liberty nor safety."
                 - Benjamin Franklin


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Parameters & thread safety

Posted by Klaus Sonnenleiter <kl...@micromuse.com>.
I think it should be up to the developer to synchronize when necessary. 
Otherwise you'll end up with lots of overhead for synchronizing puts where 
there is possibly no need at all.

Just my $.02

Klaus Sonnenleiter

At 05:04 PM 12/18/2001 +0100, you wrote:
>Hi All,
>
>         This time with a subject line!.. :-)
>
>         Cheers,
>
>         Marcus
>
>On Tue, Dec 18, 2001 at 05:03:01PM +0100, crafterm wrote:
> > Hi All,
> >
> >       Hope all is well!
> >
> >       I've just been doing some debugging with our application here in
> >       Dresdner Bank, which has led me to have a closer look at the
> >       Parameters class. From my analysis the Parameters class is not
> >       thread safe.
> >
> >       (Parameters uses HashMap as it's internal store, and does not
> >       synchronize any of its mutator methods).
> >
> >       I'm just wondering if this is intended (ie. like HashMap in the
> >       Collections API - up to the developer to synchronize access) or
> >       whether this is should be fixed as it can currently cause grief
> >       in Cocoon under load.
> >
> >       Any thoughts/comments/etc ?
> >
> >       Cheers,
> >
> >       Marcus
> >
> > --
> >         .....
> >      ,,$$$$$$$$$,      Marcus Crafter
> >     ;$'      '$$$$:    Computer Systems Engineer
> >     $:         $$$$:   ManageSoft GmbH
> >      $       o_)$$$:   82-84 Mainzer Landstrasse
> >      ;$,    _/\ &&:'   60327 Frankfurt Germany
> >        '     /( &&&
> >            \_&&&&'     Email : Marcus.Crafter@managesoft.com
> >           &&&&.        Business Hours : +49 69 9757 200
> >     &&&&&&&:
>
>--
>         .....
>      ,,$$$$$$$$$,      Marcus Crafter
>     ;$'      '$$$$:    Computer Systems Engineer
>     $:         $$$$:   ManageSoft GmbH
>      $       o_)$$$:   82-84 Mainzer Landstrasse
>      ;$,    _/\ &&:'   60327 Frankfurt Germany
>        '     /( &&&
>            \_&&&&'     Email : Marcus.Crafter@managesoft.com
>           &&&&.        Business Hours : +49 69 9757 200
>     &&&&&&&:
>
>--
>To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
>For additional commands, e-mail: <ma...@jakarta.apache.org>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>