brandizzi / google-rfc-2445

Automatically exported from code.google.com/p/google-rfc-2445
Apache License 2.0
0 stars 0 forks source link

google-rfc-2445 and thread safety. #2

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The thread safety of google-rfc-2445 is not documented. Is it thread safe?

We are using google-rfc-2445 in a multi threaded environment and have
received the following exception:
08 Feb 2008 11:00:01,218 [ERROR] xxx.SurveillanceProcessor - Error
processing surveillance 264: java.lang.IndexOutOfBoundsException Index: 6,
Size: 3
java.lang.IndexOutOfBoundsException: Index: 6, Size: 3
    at java.util.ArrayList.RangeCheck(Unknown Source)
    at java.util.ArrayList.get(Unknown Source)
    at com.google.ical.values.IcalSchema.applyContentSchema(IcalSchema.java:108)
    at com.google.ical.values.RRuleSchema$4.apply(RRuleSchema.java:158)
    at com.google.ical.values.IcalSchema.applyContentSchema(IcalSchema.java:101)
    at com.google.ical.values.RRuleSchema$1.apply(RRuleSchema.java:81)
    at com.google.ical.values.IcalSchema.applyObjectSchema(IcalSchema.java:117)
    at com.google.ical.values.AbstractIcalObject.parse(AbstractIcalObject.java:82)
    at com.google.ical.values.RRule.<init>(RRule.java:52)
    at
com.google.ical.iter.RecurrenceIteratorFactory.parseContentLines(RecurrenceItera
torFactory.java:451)
    at
com.google.ical.iter.RecurrenceIteratorFactory.createRecurrenceIterable(Recurren
ceIteratorFactory.java:93)
    at
com.google.ical.compat.jodatime.LocalDateIteratorFactory.createLocalDateIterable
(LocalDateIteratorFactory.java:83)
    at
com.google.ical.compat.jodatime.LocalDateIteratorFactory.createLocalDateIterable
(LocalDateIteratorFactory.java:100)

This exception doesn't happen every time and it is not clear under which
conditions it happens.

I fear that google-rfc-2445 is NOT thread safe. A look at the class
com.google.ical.values.IcalSchema has confirmed my fears.

Original issue reported on code.google.com by jcro...@gmail.com on 13 Feb 2008 at 1:19

GoogleCodeExporter commented 9 years ago
Hmm.  It looks like RRuleSchema is a singleton, but shares a ruleStack which 
makes it
not reentrant.  The 3rd stack frame in the above would seem to manipulate the
ruleStack, so that's probably the cause of this exception.

I think the fix to your immediate problem is to make RRuleSchema not a 
singleton.

Are you sharing RRule or RecurrenceIterator instances across threads?

Can you tell me how many threads (10s, 100s) you typically use?  Knowing that 
would
help me put together a unittest for the parser that reproduces your problem.

Original comment by mikesamuel@gmail.com on 13 Feb 2008 at 6:48

GoogleCodeExporter commented 9 years ago
We have developed a wrapper class around the google classes: the class 
ICalendar.
Each thread has its own instance of ICalendar as a method variable.
The problem already happens with about 5 threads.

I noticed that no synchronization has been used in IcalSchema. This class holds 
an
instance variable: 
  private List<String> ruleStack = new ArrayList<String>();
The method 
  applyContentSchema (line 108, belongs to stack trace in the first comment)
manipulates the variable ruleStack, without any synchronization.

IcalSchema.applyContentSchema is used by a static variable of the class 
RRuleSchema
(see RRuleSchema, line 158, belongs to stack trace in the first comment).
This part is also not synchronized.

I think that there are the reasons why our application dies with an exception.

Below, a code snippet from our class ICalendar:
  public Date getNextDate( Date startDate ) throws ParseException
  {
    return getNextDateList( startDate, 1 ).get( 0 );
  }

  public List<Date> getNextDateList( Date startDate, int count ) throws ParseException
  {
    LocalDateIterable localDateIterable = null;
    List<Date> dateList = new LinkedList<Date>();
    String iCalString = rrule.toIcal();
    boolean isFirstDate = true;
    int index = 0;
    int limit = 0;

    if ( startDate == null )
    {
      throw new IllegalArgumentException( "startDate must not be null." );
    }

    if ( count < 1 )
    {
      throw new IllegalArgumentException( "Invalid count value: " + count );
    }

    // count + 1 because the first date returned is the startDate.
    limit = count + 1;

    localDateIterable = LocalDateIteratorFactory.createLocalDateIterable( iCalString,
        new LocalDate( startDate ), true );

    // A RRULE can contain a COUNT attribute according to RFC2445.
    // Loops until COUNT is reached or the limit is reached.
    for ( Iterator iterator = localDateIterable.iterator(); iterator.hasNext() &&
index < limit; index++ )
    {
      LocalDate date = (LocalDate) iterator.next();

      if ( isFirstDate )
      {
        // The first date is the startDate. Don't get it.
        isFirstDate = false;
      }
      else
      {
        dateList.add( date.toDateTimeAtStartOfDay().toDate() );
      }
    }

    return dateList;
  }

I hope that it helps you to resolve this bug.

Original comment by jcro...@gmail.com on 14 Feb 2008 at 8:01

GoogleCodeExporter commented 9 years ago

Original comment by mikesamuel@gmail.com on 18 Feb 2008 at 4:40

GoogleCodeExporter commented 9 years ago
Fixed in r22: http://code.google.com/p/google-rfc-2445/source/detail?r=22

Original comment by mikesamuel@gmail.com on 18 Feb 2008 at 5:44