Bug 39724 - Bug on StandardPipeline.removeValve(Valve valve) for T5.5.16+
Summary: Bug on StandardPipeline.removeValve(Valve valve) for T5.5.16+
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 5.5.16
Hardware: Other other
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 35914 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-06-05 14:01 UTC by David Gagnon
Modified: 2006-10-02 18:06 UTC (History)
1 user (show)



Attachments
The patch on the removeValve Method (1.05 KB, patch)
2006-06-05 14:03 UTC, David Gagnon
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Gagnon 2006-06-05 14:01:56 UTC
I run into this problem with T5 when tring to add/remove/add a valve to the
standard engine. -For what I understand an empty pipeline has:
basic = StandardEngineValve (For example);
first = null;

-If you add a valve you will get
basic = StandardEngineValve
first = myValve (with myValve.next = StandardEngineValve)

-If you remove the valve you will get
basic =StandardEngineValve
first = StandardEngineValve
Note that StandardEngineValve is in first too here.

-If I try to add a new valve given the actual code in  addValve the valve will
not be added because (see the **) current = basic and current.getNext() = null;

addValve(Valve valve) {
 
       // Add this Valve to the set associated with this Pipeline
       if (first == null) {
           first = valve;
           valve.setNext(basic);
       } else {
           Valve current = first;
           while (current != null) {
**                if (current.getNext() == basic) {
                   current.setNext(valve);
                   valve.setNext(basic);
                   break;
               }
               current = current.getNext();
           }
       }

   }
Comment 1 David Gagnon 2006-06-05 14:03:30 UTC
Created attachment 18402 [details]
The patch on the removeValve Method
Comment 2 Yoav Shapira 2006-06-15 19:09:35 UTC
Seems like a good suggestion: can you please submit your patch in diff -u
format?  Thank you.
Comment 3 David Gagnon 2006-06-15 19:32:11 UTC
Hi,
  Here is the diff.  My patch had a little type by the way i.e.: == instead of =

Anyway the following works

Regards
/David


diff -u StandardPipeline.java StandardPipelinePatched.java
--- StandardPipeline.java       2006-06-15 15:31:12.637965896 -0400
+++ StandardPipelinePatched.java        2006-06-15 15:30:45.894031592 -0400
@@ -530,6 +530,9 @@
             current = current.getNext();
         }

+        // PATCH: Empty the pipeline if only the basic valve is there
+        if (first == basic) first = null;
+
         if (valve instanceof Contained)
             ((Contained) valve).setContainer(null);
Comment 4 Mark Thomas 2006-10-02 18:02:24 UTC
*** Bug 35914 has been marked as a duplicate of this bug. ***
Comment 5 Mark Thomas 2006-10-02 18:06:07 UTC
Patch applied to SVN and will be in 5.5.21 onwards.

Thanks for tracking this down and providing the patch.