[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-22 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

Felix Schumacher felix.schumac...@internetallee.de changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #12 from Felix Schumacher felix.schumac...@internetallee.de ---
This fix has been applied to tomcat7.0.x (for 7.0.58 and onwards)

Thanks for the nice debugging work.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-21 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #11 from Violeta Georgieva violet...@apache.org ---
(In reply to Felix Schumacher from comment #9)
 Created attachment 32382 [details]
 Make UEncoder a local variable and introduce a default safe charset enum
 
 This patch uses a local variable for the UEncoder in DirContextURLConnection
 and enables the safeChars in UEncoder to be immutable.
 
 That way all Response and DirContextURLConnection instances can share one
 safeChars BitSet while having their own UEncoder instance which remains
 unsafe for multithreaded usage.

+1

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-21 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #10 from Violeta Georgieva violet...@apache.org ---
(In reply to Konstantin Kolinko from comment #7)
 (In reply to Violeta Georgieva from comment #6)
  (In reply to Konstantin Kolinko from comment #4)
  
   UEncoder is used in list(). While it is possible to call list() multiple
   times, I think that in practice it is called only once.
  
  list() is called many times by
  org.apache.catalina.startup.ContextConfig.processAnnotationsJndi(URL,
  WebXml, boolean)
  
  As we add a save char to UEncoder I do not think that it will be OK to do
  this every time when list() is called.
  
  Wdyt?
 
 As far as I see, it opens a new DirContextURLConnection each time. While the
 method is called many times, a connection is not reused. Thus a class-level
 field provides no benefit.
 
You are right - +1

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-20 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #7 from Konstantin Kolinko knst.koli...@gmail.com ---
(In reply to Violeta Georgieva from comment #6)
 (In reply to Konstantin Kolinko from comment #4)
 
  UEncoder is used in list(). While it is possible to call list() multiple
  times, I think that in practice it is called only once.
 
 list() is called many times by
 org.apache.catalina.startup.ContextConfig.processAnnotationsJndi(URL,
 WebXml, boolean)
 
 As we add a save char to UEncoder I do not think that it will be OK to do
 this every time when list() is called.
 
 Wdyt?

As far as I see, it opens a new DirContextURLConnection each time. While the
method is called many times, a connection is not reused. Thus a class-level
field provides no benefit.

It should be possible to initialize UEncoder.safeChars bitset once and share
the same bitset between UEncoders.  It will save us some time and memory.

A single UEncoder.addSafeCharacter() call does not matter much, but
UEncoder().initSafeChars() that adds around 60 characters does matter.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-20 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #8 from Felix Schumacher felix.schumac...@internetallee.de ---
What if we add another constructor which sets the bitset to use (or add a
method to set it)? Or make the initial bitset static and clone that on
construction?

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-19 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #6 from Violeta Georgieva violet...@apache.org ---
(In reply to Konstantin Kolinko from comment #4)
 1) As far as I see, UDecoder does not have any fields and thus is
 thread-safe.
 
 The thread safety is not documented, though.
 
 As such, I do not mind creating an instance of UDecoder, as that feels safer.
 
 A possible way to solve documentation issue is to add getInstance() factory
 methods to UEncoder, UDecoder. For decoder the method would return a
 singleton (thus documenting the thread safety), while for encoder it will
 always return a new instance.
 
 2) Both encoder and decoder are usually used only once per connection.
 
 Thus I think it would be better to use local variables, creating the objects
 just before use, instead of caching them in class fields.
 
 UDecoder is used in connect(). That call happens once.

+1 for creating it as a local variable

 UEncoder is used in list(). While it is possible to call list() multiple
 times, I think that in practice it is called only once.

list() is called many times by
org.apache.catalina.startup.ContextConfig.processAnnotationsJndi(URL, WebXml,
boolean)

As we add a save char to UEncoder I do not think that it will be OK to do this
every time when list() is called.

Wdyt?

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-19 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #4 from Konstantin Kolinko knst.koli...@gmail.com ---
1) As far as I see, UDecoder does not have any fields and thus is thread-safe.

The thread safety is not documented, though.

As such, I do not mind creating an instance of UDecoder, as that feels safer.

A possible way to solve documentation issue is to add getInstance() factory
methods to UEncoder, UDecoder. For decoder the method would return a singleton
(thus documenting the thread safety), while for encoder it will always return
a new instance.

2) Both encoder and decoder are usually used only once per connection.

Thus I think it would be better to use local variables, creating the objects
just before use, instead of caching them in class fields.

UDecoder is used in connect(). That call happens once.

UEncoder is used in list(). While it is possible to call list() multiple times,
I think that in practice it is called only once.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-19 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

Violeta Georgieva violet...@apache.org changed:

   What|Removed |Added

  Attachment #32378|0   |1
   is patch||

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-19 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #3 from Violeta Georgieva violet...@apache.org ---
Created attachment 32378
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=32378action=edit
Patch

Hi,

What do you think if we switch to UEncoder/UDecoder as regular fields of the
class? (see attachment)

Regards,
Violeta

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-19 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #5 from Felix Schumacher felix.schumac...@internetallee.de ---
I came up with the same patch as Violeta and it fixes the reported problem
(unpackWARs has to be set to false, if you want to reproduce it and of course
it helps if some sleep statements are inserted into the encoder methods).

I agree, that it would be nicer to use the UDecoder/UEncoder as local
variables. I don't think that we should introduce getInstance methods for
tomcat7 only.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-18 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #2 from matthias.kel...@ergon.ch ---
We're starting it from command line using catalina run. The scenario I was
able to (sometimes) reproduce the problem hat two WAR files with lots of
classes inside the WEB-INF/classes directory. One thing that could also make a
difference is, that the classes of the two WAR files are very similar, 95% (or
more) are the same or have at least the same class names.

The parameters used are startStopThreads=0 (= parallel), we're not using a
virtualWebappLoader.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-17 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

--- Comment #1 from Felix Schumacher felix.schumac...@internetallee.de ---
Could you tell us, how you are starting the application?

For example: Is it inside eclipse, as a war, ...? Are you using parallel
startup with the startStopThreads parameter in Host? Do you use
virtualWebappLoader?

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 57420] Wrong class names generated since URL_ENCODER in DirContextURLConnection is not thread safe

2015-01-07 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=57420

Marc Buetikofer marc.buetiko...@ergon.ch changed:

   What|Removed |Added

 CC||marc.buetiko...@ergon.ch
 OS||All

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org