Re: [Bug 51919] Random ConcurrentModificationException or NoSuchElementException in CookieManager#removeMatchingCookies when using Concurrent Download
Hello Sebb, all,
First a note regarding
If cookies are being ignored, then the cookie manager property can just be
cleared - i.e. there is no cookie manager.
Just to be sure I understand, you mean Cookies of embedded resources are
not used, because download of embedded resources may require JSESSIONID
cookie at least or any cookie containing Session information.
In this case can CookieManager be null or shouln't it be cloned from parent
but it's result not used ? Maybe that's what you mean or I missed something.
So, regarding your last comment on 51919 here is an updated implementation
proposition:
- Add an option embedded_resources_use_cookies to use Cookies we get
from embedded resources (conc or serial) download, default value will be
true (to handle frames by default).
- Create a Bean AsynSamplerResultHolder that will hold:
- HTTPSampleResult
- Cookie[]
*Conc download:*
- Pass CookieManager to AsynSampler, clone it and add existing cookie,
and store it in ThreadLocal
- At end of sample:
- If embedded_resources_use_cookies is false =build
AsynSamplerResultHolder
with HTTPSampleResult and empty cookies array
- If embedded_resources_use_cookies is true =build
AsynSamplerResultHolder
with HTTPSampleResult and new cookies
- Inside FutureResult#get loop, merge result in CookieManager
*Serial Mode:
*
- Before start of sample, backup CookieManager
- At end of sample:
- If embedded_resources_use_cookies is false = ignore cookies
- If embedded_resources_use_cookies is true = add cookies to backup
CookieManager
Modify HTTPSamplerBase#getCookieManager to get CookieManager first from
Thread Local then from testElement. (as today)
Waiting for your comments on this.
Regards
Philippe
On Mon, Oct 3, 2011 at 5:02 PM, sebb seb...@gmail.com wrote:
On 3 October 2011 15:39, Philippe Mouawad philippe.moua...@gmail.com
wrote:
On Mon, Oct 3, 2011 at 4:31 PM, sebb seb...@gmail.com wrote:
On 3 October 2011 14:04, Philippe Mouawad philippe.moua...@gmail.com
wrote:
You are right,
Patch was just about quick fix before the more impacting fix.
Here are my propositions regarding this more impacting fix:
- Add an option to make conc download use or not cookie, default
value
will be false.
- In AsyncSampler make a Clone with current cookies of Parent
CookieManager (the one that is calling Executor) and store it in
ThreadLocal
- Change HttpSamplerBase#getCookieManager to retrieve first in
Thread
Local then in instance
- If option is true, when reading res in FutureHTTPSampleResult
loop,
reinject cookies inside ParentCookie otherwise ignore them
As I wrote earlier, I'm not sure it ever makes sense to handle cookies
from embedded resources, so it would be simpler just to ignore them.
I am not sure Frame couldn't be concerned by this case, so in my opinion
it
should be a parameter not ignored by default.
I'd overlooked frame.
I'm looking at passing the current sampler to the AsyncSampler class,
which can then clone it.
I think that will create an independent set of cookies, so there can
be no need to protect against concurrent updates.
Agree, that's how I imagined it, but then you agree you have to store
CookieManager in thread local ?
If cookies are being ignored, then the cookie manager property can
just be cleared - i.e. there is no cookie manager.
Alternatively, it will also need to be cloned.
We then need to consider if non-concurrent downloads should still be
processed for cookies.
Conc and serial should have the same behaviour
PS : Does it mean that you are working on issue and will fix it on your
side
Not yet decided; we need to agree on the approach first.
Regards
Philippe
On Mon, Oct 3, 2011 at 2:29 PM, sebb seb...@gmail.com wrote:
On 3 October 2011 13:14, Philippe Mouawad
philippe.moua...@gmail.com
wrote:
Sebb,
Do you want me to provide a patch with ConcurrentHashMap where I
will
have
to handle null keys and values (not same behaviour as HashMap) or
we
forget
about this approach ?
I don't think we have yet decided how best to handle concurrent
downloads.
e.g. should they even be setting cookies?
Regards
Philippe
On Mon, Oct 3, 2011 at 1:58 PM, Philippe Mouawad
philippe.moua...@gmail.com
wrote:
Hello,
Just a little update on my test.
I added a clear and gc before each Map instanciation and results
are
different:
- HashMapput:645
- ConcurrentHashMapput:832
- ConcurrentReaderHashMapput:620
- HashMap get:17
- ConcurrentHashMap get:32
- ConcurrentReaderHashMap get:60
So it kind of closes the debate, sorry for disturbance.
Regards
Philippe
public class TestMap {
public static void main(String[] args) {
Welcome to Jakarta and JMeter!
Welcome to Apache Jakarta and JMeter! == [This is a brain dump in no particular order and is open to discussion] First: decisions are made on the public mailing lists (or occasionally on Bugzilla issues for technical issues). Apache code is open source, and so are the related decisions. The only exception is for reports of security issues, but that is unlikely to be relevant for JMeter. Also personnel matters - election of new committers etc - happen on the PMC private list. The developer list (this list) is for development issues; usage questions should be confined to the JMeter User list. Code layout: follow the existing conventions of classes; generally 4 spaces indentation (never any tabs). We don't use @author tags in source code. Credit can be given in the changes list if required. Don't use the $Date: $ SVN tag - it is displayed in the default Locale, which causes problems when comparing source code with SVN. Make sure you have configured SVN defaults correctly, see [1] Each commit should deal with a single issue. Ideally all the updates needed to fix an issue or implement a new feature should be added in a single commit, including the documentation and test cases. However, sometimes this would result in a very large commit, in which case try to break it down into smaller self-contained commits. Remember that the commit messages may be reviewed by many people, so - use a log message that helps the reader understand the reason for the commit - don't mix different fixes in a single commit When fixing a Bugzilla issue, remember to include the Bugzilla details in the commit message, and it helps to add the SVN commit message summary to the Bugzilla issue when resolving it. SVN log messages should be regarded as temporary, as a means to understand the reason for the commit. They should not be used to document the code. ASF code is released as source code, and it should not be necessary to have to read the SVN logs to understand the code. The SVN logs might not be available to the end-user - and SVN log messages can be modified; any previous log message is lost. Change logs should not be added to source files; that's what SVN history is for. Nor do Bugzilla ids need to be added to the source code except where it helps explain the code. Code layout fixes such as tab removal and indentation fixes should generally be done in a separate commit. Javadoc can be omitted for trivial getter/setter methods, but otherwise should be present. (yes, there are lots of methods which need Javadoc still!) All new files created at the ASF should have the AL header. If you have not created the code yourself, please check that it is OK to commit it. Short patches submitted via Bugzilla are OK, but anything substantial may need additional documentation. If you want to work on a new feature that changes a lot of files, it might be worth doing so in a branch; just e-mail the dev list first to say what you are proposing. JMeter-specific issues It is important to maintain upwards compatibility of JMX files, so don't change any constants that are used to define the JMX attributes. When adding new attributes, ensure that the default value is also defined as a constant; this can then be used to ensure the attribute is omitted if using the default value. Likewise, the output sample result XML files have a specific format. These files may be read back by JMeter listeners, and may be processed by various different 3rd party tools and scripts. Changes to the fomat need to be upwards compatible or a lot of JMeter users will be seriously inconvenienced. JMeter runs in GUI, non-GUI, and in client-server mode. GUI classes should only do processing that is relevant to the GUI, or problems may occur when the test plan is run non-GUI. The JMeter code is split into several different source trees, which are combined into different jars. These are built in a specific order; care must be taken not to reference later methods from earlier classes otherwise a clean build will fail. JMeter code is 100% Java, and should run on any OS with a compatible JVM. OS-specific code should only be used to fix problems on that specific OS; code that only works on a specific OS - OS-specific extensions - are out of scope for JMeter. The Ant build script has a test target which exercises most of the code; if making a non-trivial change please run this before committing. However if you do manage to break the code, the Gump test run will find it soon enough. [1] http://www.apache.org/dev/version-control.html#https-svn-config - To unsubscribe, e-mail: dev-unsubscr...@jakarta.apache.org For additional commands, e-mail: dev-h...@jakarta.apache.org
