[JS2-457] JetspeedPipeline synchronization is broken and using a threadlocal is really ugly

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Component/s: Container
• Assignee: David Sean Taylor
• Reporter: David Jencks
• Created: 29/Dec/05 12:15
• Updated: 04/Oct/11 21:37
• Resolved: 31/Dec/05 08:49

Description

I previously addressed these issues in J2-115 and still think that is a better solution. Here is a way to remove use of a threadlocal to track a call's progress through the valves that does not change any classes or configuration other than JetspeedPipeline.

In addition, the synchronization in JetspeedPipeline is fairly useless. Some problems include:

suppose 2 calls come simultaneously and block on valves1. The first call changes valves to point to valves2. While the second call is changing valves2 to valves3, while locking valves1, a third call comes in, finds valves2 unlocked, and changes valves2 to vavles4. The result might be valves3 or valves4, but won't have both changes.

Also, the access to valves while traversing the pipeline needs to be synchronized, or there is no guarantee that a completely written vavle array will be accessed: according to arguments made around "double checked locking is broken", the JIT can rearrange code so that the "valves" variable can be set before the array is filled in.

I believe all the synchronization can be removed if the addValve and removeValve methods are eliminated. I personally think this would be a good idea.