Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
[EMAIL PROTECTED] wrote: DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG· RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=36541. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND· INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=36541 --- Additional Comments From [EMAIL PROTECTED] 2005-09-07 23:43 --- I strongly agree with Craig, et al to error on the side of a more robust implemention by using synchronization on the side of Tomcat. I think doing otherwise would be doing a high-wire act without a net. There are too many places for a developer to miss handling the threading correctly to leave this as is. Somehow, I cannot access bugzilla right now. DNS failure or something. Can someone close this as invalid and end this pointless debate ? Otherwise, I'll do it first thing when I have access :) Thanks, Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
[EMAIL PROTECTED] wrote: --- Additional Comments From [EMAIL PROTECTED] 2005-09-07 21:29 --- After we finish clarifying who's is bigger (and mine is best case average), we could return to the bug. 1. The bug isn't invalid, so I reopen it. You can set it to WONTFIX, but not to INVALID. Indeed, WONTFIX looks to me like a reasonable resolution. 2. Applications working well with tomcat 4, resin or probably any other servlet container do not work with tomcat 5.0.x or tomcat 5.5.x. Thus tomcat 5 seems to be not compatible to the servlet spec (or be the only one compatible), if not in the language of the specification itself, but in the intension of it. So tomcat 5 can be considered BROKEN. Fine with me. 3. The bug applies to tomcat 5.5.x as well, because ... added back synchrnozation on write operations to the map in the 5.5 branch... doesn't fix anything, since the read operation are the problem and MUST be synchronized. Well, actually, I think you should bring that up with Sun, which have made a questionable/unsafe implementation of the collection IMO. A reasonable fix I would consider, however (showing I am not saying no to your bug report just for fun), is using another collection implementation, or tweaking that one, whatever works. After all, all that is missing is something like an e != e.next in HashMap.get. But of course, we're the OSS project, so we're always the ones being bugged to death, rather than proprietary software vendors :( Synchronizing the writes will also fix problems, since I think the underlying structure of the hashmap went bad due to a concurrency of reads. Try it before whining. Thanks. 5. The idea of removing synchronization to gain performance is absurd. Who needs the performance? People like us, who have 2000-3000 concurrent users on each webserver. But people who have 2000-3000 concurrent users, have also many concurrent requests, even from the same user, so instead of gaining performance we are gaining CRASHES. This bug killed 8 (!!!) webservers yesterday. I have extremely few problems with this. 6. Consider the flurry in the tomcat users community, if the above points + your refusal to provide a three LOCs fix gonna make tomcat 5 UNUSEABLE. Funny. The only flurry is the one you have created, and that I will have to ignore. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
Remy Maucherat wrote: Synchronizing the writes will also fix problems, since I think the underlying structure of the hashmap went bad due to a concurrency of reads. Try it before whining. Thanks. Just a quick clarification. Did you mean to write ...Synchronizing the *reads* will also fix problems...? If concurrent reads is the problem, then don't the reads need to be synchronised? I thought from one of your earlier posts that the writes were already synchronised. Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
[EMAIL PROTECTED] wrote: I'm not even sure if I've been bitten by this either, but I have seen on the list numerous people speaking of running out of Tomcat threads and setting their connections to the max. If this issue were causing problems they might be having it and not even realize it. It's trivial to see using a thraed dump. Please, don't try lame FUD. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
Mark Thomas wrote: Just a quick clarification. Did you mean to write ...Synchronizing the *reads* will also fix problems...? If concurrent reads is the problem, then don't the reads need to be synchronised? I thought from one of your earlier posts that the writes were already synchronised. Not in the 5.0.x build he's using. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
Remy Maucherat wrote: Mark Thomas wrote: Just a quick clarification. Did you mean to write ...Synchronizing the *reads* will also fix problems...? If concurrent reads is the problem, then don't the reads need to be synchronised? I thought from one of your earlier posts that the writes were already synchronised. Not in the 5.0.x build he's using. Indeed. But do we need to sync the reads in 5.5.x as well or is it enough just to do the writes? I am confused as you said concurrent reads were the issue but syncing the writes would fix it. Sorry if I am being dense here ;) Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
[EMAIL PROTECTED] wrote: --- Additional Comments From [EMAIL PROTECTED] 2005-09-08 01:08 --- I wonder if the new java.util.concurrent classes could be used instead of simple HashMap? http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ConcurrentHashMap.html but that would mean total dependence on j2se 1.5 and that would be a problem for supporting j2se 1.4, though a backport is being worked on here http://www.mathcs.emory.edu/dcl/util/backport-util-concurrent/ other reading here: http://gee.cs.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/concurrent/intro.html I think that sort of thing would provide a nice solution for speed + reliability. Using this as the underlaying base would/should fix issues with EL access too. This would be a bad solution, because there are too many writes in many cases. From what I see, the regular HashMap.get will return null without further trouble (since that is all that is needed) as long as no resize occurs. This makes the problem fixable by creating a non resizable HashMap. Another possibility, which I consider very likely, is that concurrent resizes are the only problem, and in that case syncing only on writes is already enough. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DO NOT REPLY [Bug 36541] - session getAttribute/setAttribute and removeAttribute are NOT Thread safe.
Mark Thomas wrote: Remy Maucherat wrote: Indeed. But do we need to sync the reads in 5.5.x as well or is it enough just to do the writes? I am confused as you said concurrent reads were the issue but syncing the writes would fix it. No, concurrent reads are not the issue. The problem is that when the HashMap structure is corrupted by concurrent writes (either a put causing a resize, or a remove), then any read might loop. I believe as long as the HashMap structure remains consistent, all the next fields for the entries (which is the problematic field) will have a correct value at the end of the resize/remove, so an infinite loop on get is then not possible. What this issue tells me, also, is that using the default size for the attributes HashMap may be inappropriate (it is best to avoid resizes, and the question is how much they happen - if at all, maybe it is remove which is most often used). Sorry if I am being dense here ;) No problem :) Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
