Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 26, 2008, at 3:59 AM, Thorsten Scherler wrote: On Mon, 2008-08-25 at 20:23 -0400, Vadim Gritsenko wrote: On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: ... Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is actually unique in any environment. And since it is unique, it does not make sense to lock on it at all - lock on unique object serves no purpose :-) Makes me wonder since HttpServletRequest is always unique as well due to the headers, so does not violate as well the whole locking? HttpServletRequest is unique only in context of external request; but it is same for multiple sub-requests of single external request. Similarly, CommandLineRequest would be unique per single external request, but shared across sub-requests. At the same time, ObjectModelHelper.REQUEST_OBJECT will be always unique: it will either be HttpServletRequest/CommandLineRequest for top level, or Wrapper for nested requests. You could, for example, have same effect with code: // Avoids NPE, does nothing if (lock == null) lock = new Object(); To get back to the problem... The original goal of pipeline locking was to prevent separate external requests to start generating cached response for the same resource-intensive resource. In other words, if too external requests both (directly or through aggregation) hit '/ slow/cached/foo' resource, only one request will trigger this pipeline, while another will wait for the first to complete. This logic, in turn, had to be augmented to exclude single external request hitting the same slow resource twice due to aggregation (and parallel aggregation), hence locking against top level external request was implemented. Understand but I am confused due to the above fact that the HttpServletRequest is as well unique as it is my understanding. Now, in situation when CommandLineEnvironment is used from the, ahem, command line, two external requests will not happen. Which means that whole pipeline locking thing is irrelevant and can be omitted completely (like in 'lock = new Object()' snippet above). But, the same CommandLineEnvironment stuff can be used in multi threaded environment which uses the CocoonBean class. So, strictly speaking, in this scenario locking still should be performed against external request object, and not against any RequestWrapper which is unique for each sub-request. I now are using the url as lock key since as I understand it perfectly qualifies as lock key. Why not CommandLineRequest? The patch is attached to COCOON-2241. As soon you are ok with it I will commit it. I'd rather use CommandLineRequest; or at least change one line to be: this.objectModel.put(CLI_REQUEST_ID, new String(uri)); Vadim Thanks for your patience. salu2 Hope now it all makes more sense. Regards, Vadim In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2 -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Thu, 2008-08-28 at 07:44 -0400, Vadim Gritsenko wrote: On Aug 26, 2008, at 3:59 AM, Thorsten Scherler wrote: On Mon, 2008-08-25 at 20:23 -0400, Vadim Gritsenko wrote: On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: ... Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is actually unique in any environment. And since it is unique, it does not make sense to lock on it at all - lock on unique object serves no purpose :-) Makes me wonder since HttpServletRequest is always unique as well due to the headers, so does not violate as well the whole locking? HttpServletRequest is unique only in context of external request; but it is same for multiple sub-requests of single external request. Similarly, CommandLineRequest would be unique per single external request, but shared across sub-requests. At the same time, ObjectModelHelper.REQUEST_OBJECT will be always unique: it will either be HttpServletRequest/CommandLineRequest for top level, or Wrapper for nested requests. You could, for example, have same effect with code: // Avoids NPE, does nothing if (lock == null) lock = new Object(); To get back to the problem... The original goal of pipeline locking was to prevent separate external requests to start generating cached response for the same resource-intensive resource. In other words, if too external requests both (directly or through aggregation) hit '/ slow/cached/foo' resource, only one request will trigger this pipeline, while another will wait for the first to complete. This logic, in turn, had to be augmented to exclude single external request hitting the same slow resource twice due to aggregation (and parallel aggregation), hence locking against top level external request was implemented. Understand but I am confused due to the above fact that the HttpServletRequest is as well unique as it is my understanding. Now, in situation when CommandLineEnvironment is used from the, ahem, command line, two external requests will not happen. Which means that whole pipeline locking thing is irrelevant and can be omitted completely (like in 'lock = new Object()' snippet above). But, the same CommandLineEnvironment stuff can be used in multi threaded environment which uses the CocoonBean class. So, strictly speaking, in this scenario locking still should be performed against external request object, and not against any RequestWrapper which is unique for each sub-request. I now are using the url as lock key since as I understand it perfectly qualifies as lock key. Why not CommandLineRequest? Hmm, because it would be the same as ObjectModelHelper.REQUEST_OBJECT this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, new CommandLineRequest(this, null, uri, null, attributes, parameters, headers)); However if you recommend to do: CommandLineRequest request = new CommandLineRequest(this, null, uri, null, attributes, parameters, headers); this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, request); this.objectModel.put(CLI_REQUEST, request); That is as well fine with me. wdyt? Thanks for your feedback. salu2 The patch is attached to COCOON-2241. As soon you are ok with it I will commit it. I'd rather use CommandLineRequest; or at least change one line to be: this.objectModel.put(CLI_REQUEST_ID, new String(uri)); Vadim Thanks for your patience. salu2 Hope now it all makes more sense. Regards, Vadim In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2 -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Mon, 2008-08-25 at 20:23 -0400, Vadim Gritsenko wrote: On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: ... Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is actually unique in any environment. And since it is unique, it does not make sense to lock on it at all - lock on unique object serves no purpose :-) Makes me wonder since HttpServletRequest is always unique as well due to the headers, so does not violate as well the whole locking? You could, for example, have same effect with code: // Avoids NPE, does nothing if (lock == null) lock = new Object(); To get back to the problem... The original goal of pipeline locking was to prevent separate external requests to start generating cached response for the same resource-intensive resource. In other words, if too external requests both (directly or through aggregation) hit '/ slow/cached/foo' resource, only one request will trigger this pipeline, while another will wait for the first to complete. This logic, in turn, had to be augmented to exclude single external request hitting the same slow resource twice due to aggregation (and parallel aggregation), hence locking against top level external request was implemented. Understand but I am confused due to the above fact that the HttpServletRequest is as well unique as it is my understanding. Now, in situation when CommandLineEnvironment is used from the, ahem, command line, two external requests will not happen. Which means that whole pipeline locking thing is irrelevant and can be omitted completely (like in 'lock = new Object()' snippet above). But, the same CommandLineEnvironment stuff can be used in multi threaded environment which uses the CocoonBean class. So, strictly speaking, in this scenario locking still should be performed against external request object, and not against any RequestWrapper which is unique for each sub-request. I now are using the url as lock key since as I understand it perfectly qualifies as lock key. The patch is attached to COCOON-2241. As soon you are ok with it I will commit it. Thanks for your patience. salu2 Hope now it all makes more sense. Regards, Vadim In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2 -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Fri, 2008-08-22 at 10:34 -0400, Vadim Gritsenko wrote:
On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote:
On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote:
On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote:
How about:
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object.
Exactly this thing I was trying to explain - it will not work because
instead of using top level request you are suggesting to use current
sub-request.
Sorry I am not really following. As understand you we cannot use
ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a
sub-request (a request for the exact same resource in a concurrent
situation).
Exactly.
If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we
are in CLI mode.
(patch quoted above does not have lock == null comparison anymore,
that's why it is dangerous for HTTP env)
In CLI ASAIK there is no concurrent situation meaning
the problem cannot occur. However since the issue is talking about
includes that may occur.
Yes. You can still use IncludeTransformer in CLI. So deadlock can
happen.
Having a typical call from the cli gives for the env.getObjectModel():
HashMap values:
[null,
[EMAIL PROTECTED],
link-collection=[],
[EMAIL PROTECTED],
null, null, null, null,
context
=
org
[EMAIL PROTECTED],
null, null, source-
resolver
=
org
[EMAIL PROTECTED],
org
.apache
.cocoon
.components
.CocoonComponentManager
[EMAIL PROTECTED], null,
null, null]
So which object you would suggest to lock?
You wrote Suitable alternative would be to lock against top most
command line request.
To what you are referring with top most command line request?
That would be CommandLineRequest.
Hmm, did you see the above objectModel? There is no such an object in
the map. There are exactly six objects in the model
1) response
2) link-collection
3) request
4) context
5) source-resolver
6) org.apache.cocoon.components.CocoonComponentManager
Following HTTP environment analogy,
that would be CommandLineEnvironment.CLI_REQUEST_OBJECT.
Which does not exist, right?
I opened an issue COCOON-2241 around the issue.
The proposed solution is now taken the ObjectModelHelper.REQUEST_OBJECT
if the request is from CLI, if not it is using the
HttpEnvironment.HTTP_REQUEST_OBJECT.
IMO it should not be a problem with deadlocks since the
FileSavingEnvironment and LinkSamplingEnvironment (as only
implementations for the Cli) have in the constructor:
this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT,
new CommandLineRequest(this, null, uri, null, attributes, parameters,
headers));
which IMO makes it unique for each sub-request.
In comparison the HttpEnvironment has in the constructor:
this.objectModel.put(HTTP_REQUEST_OBJECT, req);
where HttpServletRequest req.
The uniqueness in both cases are, as I understand, the headers.
salu2
Vadim
salu2
Take a look at EnvironmentWrapper (which is used e.g. in
SitemapSource):
// create new object model and replace the request object
Map oldObjectModel = env.getObjectModel();
if (oldObjectModel instanceof HashMap) {
this.objectModel = (Map)
((HashMap)oldObjectModel).clone();
} else {
this.objectModel = new HashMap(oldObjectModel.size()*2);
Iterator entries = oldObjectModel.entrySet().iterator();
Map.Entry entry;
while (entries.hasNext()) {
entry = (Map.Entry)entries.next();
this.objectModel.put(entry.getKey(),
entry.getValue());
}
}
this.request = new
RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel),
requestURI,
queryString,
this,
rawMode);
this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT,
this.request);
Vadim
--
Thorsten Scherler
thorsten.at.apache.org
Open Source Java consulting, training and
solutions
--
Thorsten Scherler thorsten.at.apache.org
Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 22, 2008, at 11:26 AM, Rainer Pruy wrote: Hi, sorry for intruding this discussion. Not intruding at all :) just from what I can take from this discussion: Wouldn't it be easier to enclose this into a generalized Environment implementation that does support Environment.getTopRequest() and is stored with ObjectModel in place of those different REQUEST_OBJECT instances - all different? That way you could also have some getCurrentRequest() or getNthRequest(int n) or any other operation that has to deal with request environments and knowledge about can be kept more local. It is a nice suggestion but it has to be weighted in carefully with regards to all Environment implementations in 2.1 and different architecture in 2.2. Given my limited time, I'm not going to venture into that. Goal of my comments was mainly to point out to the problem, and not to suggest any particular solution. But you are certainly welcome to work out the best way on how to handle this problem :) Vadim
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: On Fri, 2008-08-22 at 10:34 -0400, Vadim Gritsenko wrote: On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote: So which object you would suggest to lock? You wrote Suitable alternative would be to lock against top most command line request. To what you are referring with top most command line request? That would be CommandLineRequest. Hmm, did you see the above objectModel? There is no such an object in the map. There are exactly six objects in the model 1) response 2) link-collection 3) request 4) context 5) source-resolver 6) org.apache.cocoon.components.CocoonComponentManager Yes, it currently does not exist. Following HTTP environment analogy, that would be CommandLineEnvironment.CLI_REQUEST_OBJECT. Which does not exist, right? Correct. At the moment, it does not exist. I opened an issue COCOON-2241 around the issue. The proposed solution is now taken the ObjectModelHelper.REQUEST_OBJECT if the request is from CLI, if not it is using the HttpEnvironment.HTTP_REQUEST_OBJECT. IMO it should not be a problem with deadlocks since the FileSavingEnvironment and LinkSamplingEnvironment (as only implementations for the Cli) have in the constructor: this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, new CommandLineRequest(this, null, uri, null, attributes, parameters, headers)); which IMO makes it unique for each sub-request. Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is actually unique in any environment. And since it is unique, it does not make sense to lock on it at all - lock on unique object serves no purpose :-) You could, for example, have same effect with code: // Avoids NPE, does nothing if (lock == null) lock = new Object(); To get back to the problem... The original goal of pipeline locking was to prevent separate external requests to start generating cached response for the same resource-intensive resource. In other words, if too external requests both (directly or through aggregation) hit '/ slow/cached/foo' resource, only one request will trigger this pipeline, while another will wait for the first to complete. This logic, in turn, had to be augmented to exclude single external request hitting the same slow resource twice due to aggregation (and parallel aggregation), hence locking against top level external request was implemented. Now, in situation when CommandLineEnvironment is used from the, ahem, command line, two external requests will not happen. Which means that whole pipeline locking thing is irrelevant and can be omitted completely (like in 'lock = new Object()' snippet above). But, the same CommandLineEnvironment stuff can be used in multi threaded environment which uses the CocoonBean class. So, strictly speaking, in this scenario locking still should be performed against external request object, and not against any RequestWrapper which is unique for each sub-request. Hope now it all makes more sense. Regards, Vadim In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Thu, 2008-08-21 at 19:40 -0400, Vadim Gritsenko wrote:
On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote:
still updating forrest to use cocoon-2.1.x and I found a problem in
the
AbstractCachingProcessingPipeline.
I am not sure whether someone is using the cocoon cli ATM. Forrest is
based around this component.
https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340
#action_12624340
I found that in
org
.apache
.cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline
line 245
Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
the lock is null which causes the NPE in the end.
I changed it the following way:
Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+if (null==lock){
+ lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
+}
try {
and now it is working as before.
Can somebody verify that it is a bug? Should I commit the patch?
It is a bug, but with proposed change you can get a deadlock under
some conditions (IIRC, when using parallels inclusion in the sitemap
which ultimately pull from the same pipeline). The idea behind that
lock is to synchronize on the main request (top most request), and not
on any of sub-request object created during aggregation. Suitable
alternative would be to lock against top most command line request.
How about:
Index:
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
===
---
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
(revision 681296)
+++
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
(working copy)
@@ -36,6 +36,7 @@
import org.apache.cocoon.caching.ComponentCacheKey;
import org.apache.cocoon.caching.PipelineCacheKey;
import org.apache.cocoon.environment.Environment;
+import org.apache.cocoon.environment.ObjectModelHelper;
import org.apache.cocoon.environment.http.HttpEnvironment;
import org.apache.cocoon.transformation.Transformer;
import org.apache.cocoon.util.HashUtil;
@@ -242,7 +243,7 @@
getLogger().debug(Lock EXISTS: ' + lockKey +
');
}
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object.
Thanks for the feedback Vadim.
salu2
Vadim
--
Thorsten Scherler thorsten.at.apache.org
Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Fri, 2008-08-22 at 09:35 +0200, Thorsten Scherler wrote:
On Thu, 2008-08-21 at 19:40 -0400, Vadim Gritsenko wrote:
On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote:
still updating forrest to use cocoon-2.1.x and I found a problem in
the
AbstractCachingProcessingPipeline.
I am not sure whether someone is using the cocoon cli ATM. Forrest is
based around this component.
https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340
#action_12624340
...
Can somebody verify that it is a bug? Should I commit the patch?
It is a bug, but with proposed change you can get a deadlock under
some conditions (IIRC, when using parallels inclusion in the sitemap
which ultimately pull from the same pipeline). The idea behind that
lock is to synchronize on the main request (top most request), and not
on any of sub-request object created during aggregation. Suitable
alternative would be to lock against top most command line request.
How about:
Index:
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
===
---
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
(revision 681296)
+++
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
(working copy)
@@ -36,7 +36,7 @@
import org.apache.cocoon.caching.ComponentCacheKey;
import org.apache.cocoon.caching.PipelineCacheKey;
import org.apache.cocoon.environment.Environment;
-import org.apache.cocoon.environment.http.HttpEnvironment;
+import org.apache.cocoon.environment.ObjectModelHelper;
import org.apache.cocoon.transformation.Transformer;
import org.apache.cocoon.util.HashUtil;
import org.apache.excalibur.source.SourceValidity;
@@ -192,7 +192,7 @@
}
// Avoid deadlock with self (see JIRA COCOON-1985).
-Object current =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object current =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
if (lock != null lock != current) {
if (getLogger().isDebugEnabled()) {
getLogger().debug(Waiting on Lock ' + lockKey +
');
@@ -242,7 +242,7 @@
getLogger().debug(Lock EXISTS: ' + lockKey +
');
}
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object. I had a look as well on COCOON-1985
and it should be compatible.
Thanks for the feedback Vadim.
salu2
--
Thorsten Scherler thorsten.at.apache.org
Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote:
How about:
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object.
Exactly this thing I was trying to explain - it will not work because
instead of using top level request you are suggesting to use current
sub-request.
Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource):
// create new object model and replace the request object
Map oldObjectModel = env.getObjectModel();
if (oldObjectModel instanceof HashMap) {
this.objectModel = (Map)((HashMap)oldObjectModel).clone();
} else {
this.objectModel = new HashMap(oldObjectModel.size()*2);
Iterator entries = oldObjectModel.entrySet().iterator();
Map.Entry entry;
while (entries.hasNext()) {
entry = (Map.Entry)entries.next();
this.objectModel.put(entry.getKey(), entry.getValue());
}
}
this.request = new
RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel),
requestURI,
queryString,
this,
rawMode);
this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT,
this.request);
Vadim
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote:
On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote:
How about:
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object.
Exactly this thing I was trying to explain - it will not work because
instead of using top level request you are suggesting to use current
sub-request.
Sorry I am not really following. As understand you we cannot use
ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a
sub-request (a request for the exact same resource in a concurrent
situation).
If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we
are in CLI mode. In CLI ASAIK there is no concurrent situation meaning
the problem cannot occur. However since the issue is talking about
includes that may occur.
Having a typical call from the cli gives for the env.getObjectModel():
HashMap values:
[null,
[EMAIL PROTECTED],
link-collection=[],
[EMAIL PROTECTED],
null, null, null, null,
[EMAIL PROTECTED], null, null, [EMAIL PROTECTED], [EMAIL PROTECTED], null,
null, null]
So which object you would suggest to lock?
You wrote Suitable alternative would be to lock against top most
command line request.
To what you are referring with top most command line request?
salu2
Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource):
// create new object model and replace the request object
Map oldObjectModel = env.getObjectModel();
if (oldObjectModel instanceof HashMap) {
this.objectModel = (Map)((HashMap)oldObjectModel).clone();
} else {
this.objectModel = new HashMap(oldObjectModel.size()*2);
Iterator entries = oldObjectModel.entrySet().iterator();
Map.Entry entry;
while (entries.hasNext()) {
entry = (Map.Entry)entries.next();
this.objectModel.put(entry.getKey(), entry.getValue());
}
}
this.request = new
RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel),
requestURI,
queryString,
this,
rawMode);
this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT,
this.request);
Vadim
--
Thorsten Scherler thorsten.at.apache.org
Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote:
On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote:
On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote:
How about:
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object.
Exactly this thing I was trying to explain - it will not work because
instead of using top level request you are suggesting to use current
sub-request.
Sorry I am not really following. As understand you we cannot use
ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a
sub-request (a request for the exact same resource in a concurrent
situation).
Exactly.
If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we
are in CLI mode.
(patch quoted above does not have lock == null comparison anymore,
that's why it is dangerous for HTTP env)
In CLI ASAIK there is no concurrent situation meaning
the problem cannot occur. However since the issue is talking about
includes that may occur.
Yes. You can still use IncludeTransformer in CLI. So deadlock can
happen.
Having a typical call from the cli gives for the env.getObjectModel():
HashMap values:
[null,
[EMAIL PROTECTED],
link-collection=[],
[EMAIL PROTECTED],
null, null, null, null,
context
=
org
[EMAIL PROTECTED],
null, null, source-
resolver
=
org
[EMAIL PROTECTED],
org
.apache
.cocoon
.components
.CocoonComponentManager
[EMAIL PROTECTED], null,
null, null]
So which object you would suggest to lock?
You wrote Suitable alternative would be to lock against top most
command line request.
To what you are referring with top most command line request?
That would be CommandLineRequest. Following HTTP environment analogy,
that would be CommandLineEnvironment.CLI_REQUEST_OBJECT.
Vadim
salu2
Take a look at EnvironmentWrapper (which is used e.g. in
SitemapSource):
// create new object model and replace the request object
Map oldObjectModel = env.getObjectModel();
if (oldObjectModel instanceof HashMap) {
this.objectModel = (Map)
((HashMap)oldObjectModel).clone();
} else {
this.objectModel = new HashMap(oldObjectModel.size()*2);
Iterator entries = oldObjectModel.entrySet().iterator();
Map.Entry entry;
while (entries.hasNext()) {
entry = (Map.Entry)entries.next();
this.objectModel.put(entry.getKey(),
entry.getValue());
}
}
this.request = new
RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel),
requestURI,
queryString,
this,
rawMode);
this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT,
this.request);
Vadim
--
Thorsten Scherler
thorsten.at.apache.org
Open Source Java consulting, training and
solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
Hi,
sorry for intruding this discussion.
just from what I can take from this discussion:
Wouldn't it be easier to enclose this into a generalized Environment
implementation
that does support Environment.getTopRequest() and is stored with ObjectModel in
place of those different REQUEST_OBJECT instances -
all different?
That way you could also have some getCurrentRequest() or getNthRequest(int n)
or any other operation that has to deal with request
environments and knowledge about can be kept more local.
Just my 0,02 EUR
Rainer
Vadim Gritsenko schrieb:
On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote:
On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote:
On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote:
How about:
} else {
-Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
This way we always lock the same object.
Exactly this thing I was trying to explain - it will not work because
instead of using top level request you are suggesting to use current
sub-request.
Sorry I am not really following. As understand you we cannot use
ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a
sub-request (a request for the exact same resource in a concurrent
situation).
Exactly.
If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we
are in CLI mode.
(patch quoted above does not have lock == null comparison anymore,
that's why it is dangerous for HTTP env)
In CLI ASAIK there is no concurrent situation meaning
the problem cannot occur. However since the issue is talking about
includes that may occur.
Yes. You can still use IncludeTransformer in CLI. So deadlock can happen.
Having a typical call from the cli gives for the env.getObjectModel():
HashMap values:
[null,
[EMAIL PROTECTED],
link-collection=[],
[EMAIL PROTECTED],
null, null, null, null,
[EMAIL PROTECTED],
null, null,
[EMAIL PROTECTED],
[EMAIL PROTECTED],
null, null, null]
So which object you would suggest to lock?
You wrote Suitable alternative would be to lock against top most
command line request.
To what you are referring with top most command line request?
That would be CommandLineRequest. Following HTTP environment analogy,
that would be CommandLineEnvironment.CLI_REQUEST_OBJECT.
Vadim
salu2
Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource):
// create new object model and replace the request object
Map oldObjectModel = env.getObjectModel();
if (oldObjectModel instanceof HashMap) {
this.objectModel = (Map)((HashMap)oldObjectModel).clone();
} else {
this.objectModel = new HashMap(oldObjectModel.size()*2);
Iterator entries = oldObjectModel.entrySet().iterator();
Map.Entry entry;
while (entries.hasNext()) {
entry = (Map.Entry)entries.next();
this.objectModel.put(entry.getKey(), entry.getValue());
}
}
this.request = new
RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel),
requestURI,
queryString,
this,
rawMode);
this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT,
this.request);
Vadim
--
Thorsten Scherler thorsten.at.apache.org
Open Source Java consulting, training and solutions
[2.1] AbstractCachingProcessingPipeline and cocoon CLI
Hi all,
still updating forrest to use cocoon-2.1.x and I found a problem in the
AbstractCachingProcessingPipeline.
I am not sure whether someone is using the cocoon cli ATM. Forrest is
based around this component.
https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340#action_12624340
I found that in
org.apache.cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline
line 245
Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
the lock is null which causes the NPE in the end.
I changed it the following way:
Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+if (null==lock){
+ lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
+}
try {
and now it is working as before.
Can somebody verify that it is a bug? Should I commit the patch?
TIA for any feedback.
salu2
--
Thorsten Scherler thorsten.at.apache.org
Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote:
still updating forrest to use cocoon-2.1.x and I found a problem in
the
AbstractCachingProcessingPipeline.
I am not sure whether someone is using the cocoon cli ATM. Forrest is
based around this component.
https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340
#action_12624340
I found that in
org
.apache
.cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline
line 245
Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
the lock is null which causes the NPE in the end.
I changed it the following way:
Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+if (null==lock){
+ lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
+}
try {
and now it is working as before.
Can somebody verify that it is a bug? Should I commit the patch?
It is a bug, but with proposed change you can get a deadlock under
some conditions (IIRC, when using parallels inclusion in the sitemap
which ultimately pull from the same pipeline). The idea behind that
lock is to synchronize on the main request (top most request), and not
on any of sub-request object created during aggregation. Suitable
alternative would be to lock against top most command line request.
Vadim
