• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Thread-safe issues

 
Edmund Yong
Ranch Hand
Posts: 164
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am currently working in my second consultancy company. So far, in these two companies, I've seen code that:

(1) uses instance variables in servlets.
(2) shares attributes in a session object without synchronization on the session object.

As someone who has passed SCWCD, I know that they are bad ideas. Or is it really bad ideas? How come nobody points out about it? Are they lucky that the web sites are not accessed concurrently by many people?
 
Paul Bourdeaux
Ranch Hand
Posts: 783
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Using session objects without synchronization is technically not thread safe, as we all know... Well written code will synchronize on the HttpSession when working with sessin objects. However, not doing this will really only cause problems when the same client opens up two browser windows at the same time... so it doesn't matter how many people are using the site concurrently. It is not uncommon to see this, and the problem doesn't rear its ugly head except in rare circumstances. But it is still aproblem, and probably should be addressed. Now, trying to convince a company that they need to spend development $$$ to refactor already working code to catch the unlikely event that a client corrupts his session data by having multiple windows open... well that may be harder than the actual fix.

The instance variable problem is a little more serious. If you have multiple clients making requests concurrently, then they all have access to the servlet's instance variables at the same time. This can blow up quickly, and if there is any variable modification going on in your code (not just getting), then I am surprised this ahsn't blown up on you sooner!
 
Edmund Yong
Ranch Hand
Posts: 164
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Paul,

Thanks for your comments! Currently, in the project I'm in, I came across code that uses instance variables in servlets, like the following example:

 
Paul Bourdeaux
Ranch Hand
Posts: 783
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Interesting code.

From the looks of it, I would say that the code posted is relatively safe. The only time the x pointer is modified is in the init, which should only be called once in a servlet's lifetime. (I say should because someone could always put a call in init() in their code - you aren't supposed to, but you can...) The only possible issue I can think of is if multiple GET requests are made to the servlet before the hashtable has been completely populated from the stream, resulting in different outputs for each request.

My guess would be that this code is left over from an earlier version of J2EE, before ServletContextListeners were used and the infamous <load-on-startup> tag was applied to servlets that should be started immediately when the application initialized. IMHO, I would say that the x thread should be instantiated and started in a context listener, and then placed in an attribute for servlets to grab.
 
Edmund Yong
Ranch Hand
Posts: 164
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Paul,

Thanks for your comments. If you look at the X class, you'd notice that its run() method is getting data from a socket periodically (in fact, about once in 10 seconds). So, if there is a GET request, and x.getSomething() is called while the hashtable is still getting filled up in the x.run() method, there is a chance that the hashtable is being read and written to at the same time.
 
Paul Bourdeaux
Ranch Hand
Posts: 783
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
True, but a Hashtable (unlike a HashMap) is synchronized internally, so this shouldn't pose a fatal problem.

However, you could run into a dirty data issue. If you are populating the table with several put() calls, then it is entirely possible that another thread could make a get() call right in the middle of the populating sequence, resulting in incomplete or incorrect data. Hashtable does have a putAll() method that takes a map as an arguement, but I don't know if this method call is executed as one synchronized block, or if it just makes several internal calls to the synchronized put() method.

I would test the dirty data issue... maybe mocking the functionality of the servlet and calling it a few thousand times while the hashtable is being populated in another thread.
 
Vishnu Prakash
Ranch Hand
Posts: 1026
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

If you are populating the table with several put() calls, then it is entirely possible that another thread could make a get() call right in the middle of the populating sequence, resulting in incomplete or incorrect data.


I didn't get u here. How is it possible to read and write at the same time in a hashtable, which is synchronized.
 
Paul Bourdeaux
Ranch Hand
Posts: 783
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It is impossible to sychronize the class itself. When we say that Hashtable is synchronized, what we really mean is that the key methods in Hashtable are synchronized. So while we know that a put call and a get call cannot execute concurrently, we can't guarantee what order they will be called in. Take the following example:

Thread A is populating Hashtable H with the following calls:Meanwhile, Thread B is retrieving values with the following code:The expected output from Thread B would either be 1, 2, 3 or 4, 5, 6. However, if Thread A began its update sequence in the middle of Thread B's retrieval sequence, we could get a sequence like 1, 2, 6. This is what I am refering to as Dirty Data. The method call order for this scenario is below:
1. Thread A - put("one", new Integer(1))
2. Thread A - put("two", new Integer(2))
3. Thread A - put("three", new Integer(3))
4. Thread B - get("one")
5. Thread A - put("one", new Integer(4))
6. Thread B - get("two")
7. Thread A - put("two", new Integer(5))
8. Thread A - put("three", new Integer(6))
9. Thread B - get("three")
As you can see, none of the methods are called concurrently, yet we still have dirty data.

The solution to this, at least when using Hashtable, is to ensure that you only retrieve data from the Hashtable using an iterator. A Hashtable's iterator is fail-fast, meaning that if the Hashtbale's structure is changed at outside the Iterator itself, it will throw a ConcurrentModificationException the next time you call a method on the iterator. While this is great on paper, it is not often used in practice, becaust it is not always desirable to iterate through a Huge Hashtable just to get one value (Heck... that's why we use a Hashtable!).

Hope that helped. If not, please ask again!
 
Edmund Yong
Ranch Hand
Posts: 164
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Paul, for your explanations. The servlet will actually write all the data in the hashtable back to the client. What would be the best way to tackle this dirty read problem? Synchronize on the hashtable?
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic