To use or not to use SingleThreadModel?

Hi there,

I wanted to get input on a matter of thread safety for a servlet
that processes online tests. The basic question is, "Should I implement
the SingleThreadModel on the servlet since many students maybe
trying to submit their tests symultaneously for processing?" The servlet
does contain three instance variables which perhaps need to remain as such
so that an inner class and statements in the doPost(...) method
have access to them. Please advise if changes are necessary.

Servlet Structure and Business Logic:

The servlet contains two inner classes named Answer and XMLAnswerHandler.
A backend database contains the answers
to the tests in the form of xml markup. To process an online test,
in the doPost(...) method, I read in the xml data from the database
using the XMLAnswerHandler class which implements
org.xml.sax.helpers.DefaultHandler. XMLAnswerHandler
is used to create an instance variable ArrayList named "answers" to
contain multiple instances of class Answer. Class Answer contains
an item identifier and the actual answers to a particular test
question (item). Also, in the doPost(...) method I load up another
instance variable ArrayList named "studentAnswers" again with
multiple instances of class Answer. I then, compare the answers
given by the student with the answers in the answers ArrayList to
determine a final score.

Regards,

Alan

The servlet is as follows:

package mvcs;
 
import javax.servlet.*;
import javax.servlet.http.*;
import java.io.*;
import java.util.*;
import org.xml.sax.*;
import org.xml.sax.helpers.*;
 
 
public class TestProcessor extends HttpServlet
{
    
 
    private static ResourceBundle sql_bundle = ResourceBundle.getBundle("mvcs.testprocessor.URLs");
 
    public final static String rootElementName = "test";
    public final static String testIDElementName = "testid";
    public final static String TestTitleElementName = "title";
    public final static String QuestionCountElementName = "qcount";
    public final static String AnswersElementName = "answers";
    public final static String AnswerElementName = "answer";
    public final static String ItemElementName = "item";
    public final static String ValueElementName = "value";
    public final static String DTDURL = sql_bundle.getString("dtd"); 
    public final static String StyleURL = sql_bundle.getString("style");
 
    private ArrayList<Answer> answers = null;
    private ArrayList<Answer> studentAnswers = null;
    private int QuestionCount = 0;
      
    
 
    public void doGet(HttpServletRequest req, HttpServletResponse res)throws ServletException, IOException
    {
       res.setContentType("text/html");
       PrintWriter out = res.getWriter();
       out.println(ServletUtilities.headWithTitle("Access Denied") +
                    "<body>\n" +
                    "<h1>ACCESS DENIED</h1>\n" +
                    "</body>\n</html>");
    }
 
    public void doPost(HttpServletRequest req, HttpServletResponse res)throws ServletException, IOException
    {
       String role = "";
       String instructorID = "";
       String studentID = "";
       String testID = "";
       String title = "";
       String companyID = "";
 
       role = req.getParameter("role");
       
       if(role == null || role.equals(""))
       {
         res.setContentType("text/html");
         PrintWriter out = res.getWriter();
         out.println(ServletUtilities.headWithTitle("Access Denied") +
                    "<body>\n" +
                    "<h1>ACCESS DENIED</h1>\n" +
                    "</body>\n</html>");
         return;
       }
 
       if(role.equals("instructor"))
       {
         ...
         ...
       }
       else if(role.equals("student"))
       {
         companyID = req.getParameter("companyID");
         studentID = req.getParameter("studentID");
         testID = req.getParameter("testID");
         int passMark = TestsModel.getPassMark(testID);
 
         if(studentID == null || testID == null)
         {
           res.setContentType("text/html");
           PrintWriter out = res.getWriter();
           out.println(ServletUtilities.headWithTitle("Access Denied") +
                    "<body>\n" +
                    "<h1>ACCESS DENIED</h1>\n" +
                    "</body>\n</html>");
           return;
         }
 
         if(companyID == null)
         {
           res.setContentType("text/html");
           PrintWriter out = res.getWriter();
           out.println(ServletUtilities.headWithTitle("Access Denied") +
                    "<body>\n" +
                    "<h1>ACCESS DENIED</h1>\n" +
                    "</body>\n</html>");
           return;
         }
 
         /* If a student is transmitting a test, we will use the XML file
          * of answers, provided by the instructor, to tabulate a score. 
          * The score will be stored in the database and displayed back to
          * the student for immediate feedback.
          */
 
         //Check to make sure the studentID is a valid one for this company.
         //This has to do with the StudentID not the EmployeeID!
         boolean isValidID = StudentModel.isValidStudentID(companyID, studentID);
         if(!isValidID)
         {
           res.setContentType("text/html");
           PrintWriter out = res.getWriter();
           out.println(ServletUtilities.headWithTitle("Invalid StudentID") +
                    "<body>\n" +
                    "<h1>Invalid Student Identification</h1>\n" +
                    "<p>It has been determined that the student identification provided with the test is invalid.</p>" +
                    "</body>\n</html>");
           return;
         }
 
         //If we make it this far, everything checks out.
 
         try
         {
           //Build an ArrayList with Answer objects
           String xmlString = TestsModel.getAnswers(testID);
           //System.out.println(xmlString);
           XMLReader parser = XMLReaderFactory.createXMLReader("org.apache.xerces.parsers.SAXParser");
           org.xml.sax.ContentHandler handler = new XMLAnswerHandler();
           parser.setContentHandler(handler);
           InputSource source = new InputSource(new StringReader(xmlString));
           parser.parse(source);
 
           Answer temp = null;
           
           /*PRINT           
           for(int i = 0; i < answers.size(); i++)
           {
             temp = answers.get(i);
             System.out.println(temp.getItem() + ": ");
             String[] values = temp.getAnswers();
             System.out.println("values.length is " + values.length);
             for(int j = 0; j < values.length; j++)
             {
               System.out.println("\t" + values[j]);
             }
           }
           */
 
           //Load up the students' answers into studentAnswers ArrayList
           studentAnswers = new ArrayList<Answer>();
           Enumeration paramNames = req.getParameterNames();
           String paramName = "";
           String[] paramValues = null;
           while(paramNames.hasMoreElements())
           {
             paramName = (String)paramNames.nextElement();
             if(paramName.charAt(0) == 'Q')
             {
               paramValues = req.getParameterValues(paramName);
               //NOTE that we DO NOT filter any illegal
               //characters from the students' answers.
               //The instructors' answers have been reverse_filtered
               //during the xml parsing process, therefore,
               //we'll be comparing the raw text from both sources.
               for(int i = 0; i < paramValues.length; i++)
               {
                 paramValues[i] = paramValues[i].trim();
               }
               studentAnswers.add(new Answer(paramName, paramValues));             
             }
           }
           Collections.sort(studentAnswers);
 
           //Once the parsing is completed, we should have a Question Count
           System.out.println("QCount is: " + QuestionCount);
 
           //At this point, we have both the instructors' answers
           //and the students' answers.  We need to compare the students'
           //answers with the instructors' answers and allocate one point
           //for every correct answer.
 
           Answer instructorsAnswer = null;
           Answer studentsAnswer = null;
           String item = "";
           String[] values1 = null;
           String[] values2 = null;
           for(int i = 0; i < answers.size(); i++)
           {
             instructorsAnswer = answers.get(i);             
             item = instructorsAnswer.getItem();
             values1 = instructorsAnswer.getAnswers();
             //Iterate over the ArrayList of the students' answers
             //to get a handle on each item.
             for(int j = 0; j < studentAnswers.size(); j++)
             {
               studentsAnswer = studentAnswers.get(j);
               if(studentsAnswer.getItem().equals(item))
                  break;
             }
             //If after iterating over the studentAnswers ArrayList,
             //there isn't a match for the particular item in question,
             //then we'll just continue onto the next question item.
             if(!studentsAnswer.getItem().equals(item))
                continue;
 
             //Get the students answers
             values2 = studentsAnswer.getAnswers();
 
             /*
               Compare the answers from each array values1 and values2
               Scenarios: 
                          1) We have two arrays of length one, just compare each value.
                          2) We have two arrays of multiple lengths but should be the 
                             the same lengths. If values2 does not have as many elements.
                             or has more elements as values1, then automatically the
                             student has this one wrong!
                             However, if they are the same length, then we need to compare
                             the answers more closely.
 
             */
 
              int answersCorrect = 0;
              int answersWrong = 0;
              int score = 0; //as a percentage out of 100%
 
              if(values1.length == 1 && values2.length == 1)
              {
                if(values1[0].equals(values2[0]))
                   answersCorrect++;
              }
              else if(values1.length > 1 && values1.length == values2.length)
              {
                String val1 = "";
                String val2 = "";
                boolean correct = true;
                for(int k = 0; k < values1.length; k++)
                {
                  val1 = values1[k];
                  for(int l = 0; l < values2.length; l++)
                  {
                    val2 = values2[l];
                    if(val1.equals(val2))
                       break;
                  }
                  if(!val1.equals(val2))
                  {
                     correct = false;
                     break;
                  }
                }
                if(correct)
                   answersCorrect++;
              }             
              
           }//end of for loop
 
           //At this point, we can determine the score
           answersWrong = QuestionCount - answersCorrect;
           score = Math.round((answersCorrect*100)/QuestionCount);
           System.out.println("Number of Questions: " + QuestionCount + "\nAnswers Correct: " 
+ answersCorrect + "\nAnswers Incorrect: " + answersWrong 
+ "\nScore: " + score + "%");
 
           //reset variables
           QuestionCount = 0;
           answersCorrect = 0;
           answersWrong = 0;
           score = 0;
          
                      
         }
         catch(SAXException saxe){saxe.printStackTrace();}
         
       }       
             
   }
 
   class Answer implements Comparable<Answer>
   {
      private String item = "";
      private String[] answers = null;
      public Answer(){}
      public Answer(String ITEM, String[] ANSWERS)
      {
        if(ITEM.charAt(0) != 'Q')
           throw new IllegalArgumentException("Items must be prefixed with the character 'Q'");
        this.item = ITEM;
        this.answers = ANSWERS;
      }
      public String getItem()
      {
        return item;
      }
      public String[] getAnswers()
      {
        return answers;
      }
      public void setItem(String value)
      {
        item = value;
      }
      public void setAnswers(String[] values)
      {
        answers = values;
      }
      public int compareTo(Answer ans)
      {
        int thisItem = -1;
        int otherItem = -1;
        try
        {
          thisItem = Integer.parseInt(item.substring(1,item.length()));
          otherItem = Integer.parseInt(ans.getItem().substring(1,ans.getItem().length()));
        }
        catch(NumberFormatException nfe){nfe.printStackTrace();}
        if(thisItem == otherItem)
           return 0;
        else if(thisItem > otherItem)
           return 1;
        return -1;
      }
      public String toString()
      {
        String ans = "";
        for(int i = 0; i < answers.length; i++)
        {
           ans += answers[i] + "\n\t";
        }
        return "\nITEM: " + item + "\nANSWER: " + ans;
      }
    }
 
   class XMLAnswerHandler extends DefaultHandler
   {
      boolean haveQCountElement = false;
      boolean haveItemElement = false;
      boolean haveValueElement = false;
      Answer temp = null;
      ArrayList<String> values = null;
      StringBuffer buffer = new StringBuffer();
 
      public void startDocument()throws SAXException
      {
        answers = new ArrayList<Answer>();
      }
 
      public void startElement(String namespaceURI, String localName, String qualifiedName, Attributes atts)throws SAXException
      {
        if(localName.equals(QuestionCountElementName))
           haveQCountElement = true;
        else if(localName.equals(ItemElementName))
        {
           if(values != null)
           {
             String[] ans = new String[values.size()];
             for(int i = 0; i < values.size(); i++)
             {
               ans[i] = values.get(i);
             }
             temp.setAnswers(ans);
             answers.add(temp);
           }
           temp = new Answer();
           values = new ArrayList<String>();
           haveItemElement = true;
        }
        else if(localName.equals(ValueElementName))
        {
           haveValueElement = true;           
        }
      }
 
      public void endElement(String namespaceURI, String localName, String qualifiedName)throws SAXException
      {
        if(localName.equals(QuestionCountElementName))
           haveQCountElement = false;
        else if(localName.equals(ItemElementName))
           haveItemElement = false;
        else if(localName.equals(ValueElementName))
        {
           haveValueElement = false;
           values.add(buffer.toString().trim());
           buffer = new StringBuffer();
        }        
      }
 
      public void characters(char[] text, int start, int length)throws SAXException
      {
        if(haveQCountElement)
           QuestionCount = Integer.parseInt(new String(text,start,length));
        else if(haveItemElement)
        {
          temp.setItem(new String(text, start, length));            
        } 
        else if(haveValueElement)
        {
          String st = new String(text,start,length);
          buffer.append(ServletUtilities.reverse_filter(st));          
        } 
      }
 
      public void endDocument()
      {
        //Get the last answer
        if(values != null)
        {
          String[] ans = new String[values.size()];
          for(int i = 0; i < values.size(); i++)
          {
            ans[i] = values.get(i);
          }
          temp.setAnswers(ans);
          answers.add(temp);
        }
      }    
   }    
}

[EDIT by Dave to break long lines]
[ September 09, 2005: Message edited by: David O'Meara ]

The SingleThreadModule is depreciated. It actually doesn't do much to ensure thread safety, despite the name. It is basically the same as synchronizing your service methods.

Originally posted by Paul Bourdeaux:
It is basically the same as synchronizing your service methods.

Not exactly.
With SingleThreadModel (STM) a new instance is created for each request.
In the case of Tomcat servlet instances are pooled which makes STM quite fast.
In either case, each request doesn't need to wait for the previous one to finish before entering the service method, which would be the case if the service method were synchronized.

The more important is issue is that it's deprecated.
That should be enough to answer the original question, (to use or not to use).
[ September 09, 2005: Message edited by: Ben Souther ]

Just in case the message has not been clear:

Don't use it! Modify your servlet to be thread-safe.

Ok, so if SingleThreadModel is depicated, then for sure that eliminates one of my options. So, as it stands now, the servlet is not thread safe. I have those three instance variables there that need protecting!

An option came to mind and perhaps you can confirm. From what I've read, local variables are always thread safe. Would I be able to solve my problem by moving my instance variables and the class XMLAnswerHandler into the doPost(...) method? That way both would be local to the method and XMLAnswerHandler would still be able to see the variables "answers", "studentAnswers", and "QuestionCount".

Alan

Dang! I tried my own suggestion by moving the XMLAnswerHandler class into the doPost(...) method and the instance variables also. The compiler then complained that the instance variables had to be declared final in order to have the XMLAnswerHandler class use them. However, that makes the variables read-only. That's no good for me. Its the XMLAnswerHandler class that is supposed to set those variables!

Please advise,

Alan

I haven't gone through all of your code, so I might have misunderstood what you are trying to do. My understanding is that you want to parse the XML, and get a list of answers and studentAnswers, right? If yes, you could move the answers and studentAnswers inside your XMLAnswerHandler. Like this

class XMLAnswerHandler extends DefaultHandler
   {
[B]
      private ArrayList<Answer> answers = null;
      private ArrayList<Answer> studentAnswers = null;
      private int QuestionCount = 0;
      
      public ArrayList<Answer> getAnswers ()
      {
           return answers;
      }
      
      public ArrayList<Answer> getStudentAnswers  ()
      {
           return studentAnswers ;
      }
      
      public int getQuestionCount  ()
      {
           return QuestionCount ;
      }
[/B]
 
      boolean haveQCountElement = false;
      boolean haveItemElement = false;
      boolean haveValueElement = false;
      Answer temp = null;
      ArrayList<String> values = null;
      StringBuffer buffer = new StringBuffer();
 
      public void startDocument()throws SAXException
      {
        answers = new ArrayList<Answer>();
      }
 
      public void startElement(String namespaceURI, String localName, String qualifiedName, Attributes atts)throws SAXException
      {
        if(localName.equals(QuestionCountElementName))
           haveQCountElement = true;
        else if(localName.equals(ItemElementName))
        {
           if(values != null)
           {
             String[] ans = new String[values.size()];
             for(int i = 0; i < values.size(); i++)
             {
               ans[i] = values.get(i);
             }
             temp.setAnswers(ans);
             answers.add(temp);
           }
           temp = new Answer();
           values = new ArrayList<String>();
           haveItemElement = true;
        }
        else if(localName.equals(ValueElementName))
        {
           haveValueElement = true;           
        }
      }
 
      public void endElement(String namespaceURI, String localName, String qualifiedName)throws SAXException
      {
        if(localName.equals(QuestionCountElementName))
           haveQCountElement = false;
        else if(localName.equals(ItemElementName))
           haveItemElement = false;
        else if(localName.equals(ValueElementName))
        {
           haveValueElement = false;
           values.add(buffer.toString().trim());
           buffer = new StringBuffer();
        }        
      }
 
      public void characters(char[] text, int start, int length)throws SAXException
      {
        if(haveQCountElement)
           QuestionCount = Integer.parseInt(new String(text,start,length));
        else if(haveItemElement)
        {
          temp.setItem(new String(text, start, length));            
        } 
        else if(haveValueElement)
        {
          String st = new String(text,start,length);
          buffer.append(ServletUtilities.reverse_filter(st));          
        } 
      }
 
      public void endDocument()
      {
        //Get the last answer
        if(values != null)
        {
          String[] ans = new String[values.size()];
          for(int i = 0; i < values.size(); i++)
          {
            ans[i] = values.get(i);
          }
          temp.setAnswers(ans);
          answers.add(temp);
        }
      }    
   }    
}

Then in your doPost, you do like this

Does that work for ya?
[ September 09, 2005: Message edited by: Jayesh Lalwani ]

Originally posted by Ben Souther:

Not exactly.
With SingleThreadModel (STM) a new instance is created for each request.

You are, of course, correct Ben.

This is actually the second time you have corrected me on that... you would think I would remember sooner or later! Thanks for clarifying for me.

Hi Jayesh,

You were on the right track. I wound up making the inner class not an inner class. I just made it a normal class and loaded the variables inside it and provided accessor methods like you suggested. The main objective was to try to keep all my variables local to the doPost() method for the sake of thread safety.

Thanks for your help,

Alan