• Bug
  • Status: Open
  • 2 Major
  • Resolution:
  • Byte Code Transform
  • prodmgmt
  • Reporter: teck
  • February 01, 2008
  • 1
  • Watchers: 3
  • March 19, 2010

Description

It is easy to recreate this – Share a virgin instance of GregorianCalendar (one that is newly constructed, but for which no methods have been called) and call get(YEAR) on it.

One thought for resolving this issue, is to have an applicator call call Calendar.complete() when new instances are become shared. A review of the internal state for like issues need to be done before this approach should be implemented.

This is the [unexpected] UnlockedSharedObjectException you will see: Caused by Thread: http-0.0.0.0-8080-1 in VM(0) Shared Object Type: [I ******************************************************************************* at com.tc.object.tx.ClientTransactionManagerImpl.getTransaction(ClientTransactionManagerImpl.java:278) at com.tc.object.tx.ClientTransactionManagerImpl.fieldChanged(ClientTransactionManagerImpl.java:577) at com.tc.object.TCObjectImpl.objectFieldChanged(TCObjectImpl.java:320) at com.tc.object.TCObjectImpl.intFieldChanged(TCObjectImpl.java:360) at com.tc.object.bytecode.hook.impl.ArrayManager.intArrayChanged(ArrayManager.java:203) at com.tc.object.bytecode.ManagerUtil.intArrayChanged(ManagerUtil.java:847) at java.util.Calendar.setFieldsComputed(Calendar.java:1576) at java.util.GregorianCalendar.computeFields(GregorianCalendar.java:1999) at java.util.Calendar.complete(Calendar.java:1522) at java.util.Calendar.get(Calendar.java:1126)

Comments

Scott Bale 2008-04-03

I’ve run into a problem with Tim’s recommended resolution. In JDK 1.5, Calendar.complete() method always updates some internal fields. If complete() had been called before, any subsequent calls to complete will still (redundantly) call setFieldsComputed() and set the internal int[] stamp and boolean[] isSet arrays. In my testing I’m seeing that these sets are not even changing any values, but TC is still detecting the sets and is throwing the UnlockedSharedObjectException. (Looks like Calendar.complete() in JDK 1.4 does not have this problem.)

Tim Eck 2008-04-03

So where is the call to complete() happening? I’d think it would be called in two places; (1) Once when a calendar instance first becomes shared, (2) Once when a calendar instance is faulted into memory. In both of those case, I would think it wouldn’t really be “shared” yet, and thus wouldn’t throw an exception.

I haven’t read that class enough (it is isn’t easy to do), but perhaps some of that stuff can be transient?

Scott Bale 2008-04-05

I’ve locally modified CalendarApplicator to handle your (2) case above, calling complete() on the new Calendar instance as it is being hydrated and faulted into memory. I’m not sure if (1) is being done anywhere, I don’t think that matters for my test because it’s testing the faulted-in Calendar instance.

But the UnlockedSharedObjectException is still happening later in my test. I should’ve been clear earlier, the test is deliberately calling calendar.read(…) (on the newly hydrated instance) without locking on the calendar. Because the read(…) method calls complete(), and because complete() always calls setFieldsComputed() in JDK 1.5 and sets some fields, the UnlockedSharedObjectException gets thrown at that point.

Tim I’ll e-mail you an Eclipse patch so you can see my changes if you wish.

Scott Bale 2008-04-08

Tim, Alex and I met today about this issue, and we decided that there’s enough potential work here that we would like some outside feedback on the priority of this issue.

After examining the java.util.Calendar source code (JDK 1.5), it appears Calendar instances are not safe for concurrent access. There are many fields which are neither final nor volatile, and there is no synchronization anywhere in the class (except on one static method).

One option we discussed is instrumenting the class so that read-only methods never set values on any fields. The state machine is rather complicated so it’s hard to tell if this is possible, but the one test I wrote shows that the computeFields() method needlessly sets some fields that are already set to the same value - if this could be prevented then the UnlockSharedObjectException wouldn’t happen in that case. The computeFields() method is called in different cases though so it’s hard to tell if we could instrument the class so that read methods never have to set fields.

Another option is we could auto-synchronize on some methods (tbd) of Calendar to force it to be thread-safe, and then we could auto-lock those methods by default in tc-config. The downside of this is that even non-clustered, thread-confined instances of Calendar would have that synchronization overhead.

Finally Tim pointed out that we could do this in a new Calendar TIM, and document very clearly that if you want thread-safe clusterable Calendar instances, you can use this TIM but all instances of Calendar will have synchronization. Otherwise, user should manually guard and configure TC locks for their Calendar instances.

One last possibility that didn’t occur to me until after the meeting - could we modify TC to not throw UnlockSharedObjectException in the case that a set is being done to a field that is already set to the same value?

Scott Bale 2008-04-09

Forgot one other possibility we discussed - we could switch Calendar to logical management rather than physical management (like we do with TreeMap). But that has its own set of hurdles especially for subclassers of Calendar.

Stephen Goldbaum 2008-05-02

For some outside feedback: A lot of financial applications with large existing code bases use Calendar extensively, which makes this a major issue for adopting TC. The Calendar TIM solution looks like a good option. Another option would be to provide your own implementation of Calendar & GregorianCalendar (and TimeZone?) in the boot jar with the appropriate modifications to make it behave for TC.