[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[IDV #GQN-392793]: Fwd: [netcdf-java] DateUtil vs DateUnit time zone?
- Subject: [IDV #GQN-392793]: Fwd: [netcdf-java] DateUtil vs DateUnit time zone?
- Date: Tue, 31 Jan 2012 17:14:08 -0700
>
>
> -------- Original Message --------
> Subject: [netcdf-java] DateUtil vs DateUnit time zone?
> Date: Mon, 23 Jan 2012 17:30:33 +0100
> From: Martin Desruisseaux <address@hidden>
> Organization: Geomatys
> To: address@hidden
>
>
>
> Hello all
>
> The methods from ucar.nc2.units.DateUnit seem to assume a GMT timezone, while
> the methods in ucar.unidata.util.DateUtil seem to expect a mix of GMT timezone
> (getTimeAsISO8601, roundByDay, parseRelative) or local time zone (parse,
> findFormatter). Is it the intended working? If so, maybe it would be worth to
> document in the javadoc?
>
> As a side note, looking at the DateUtil code, I saw a few details that may be
> worth to mention. Around lines 255 and 450 there is code like below:
>
> if ((lastSdf != null)&& (lengthLastDate == dateString.length())) {
> try {
> synchronized (lastSdf) {
> lastSdf.parse(dateString);
> return lastSdf;
> }
> } catch (ParseException pe) {}
> }
>
The logic in the parse api is pretty clear and safe. The other api is not used.
Not sure if there is anything can be done at this moment to address this
concern.
Yuan
>
> This code is actually unsafe, since there is no guarantee that the lastSdf and
> lengthLastDate field values are not modified between the if statement, the
> synchronized statement and the call to the parse method (those fields are
> modified elsewhere in unsynchronized code). A partial fix would be declare the
> lastSdf field as volatile and add the following line before the above code:
>
> final SimpleDateFormat lastSdf = this.lastSdf;
>
>
> We still have a synchronization hole with lengthLastDate however. Note also
> that
> the following code (line 168):
>
> Calendar cal = Calendar.getInstance(TIMEZONE_GMT);
> cal.setTimeInMillis(time);
> Date curSysDate = cal.getTime();
>
>
> Could be replaced by a plain new Date(time), since the Date(long) constructor
> is
> timezone-insensitive. I mention just for information (not asking for changes).
>
> Martin
>
> _______________________________________________
> netcdf-java mailing list
> address@hidden
> For list information or to unsubscribe, visit:
> http://www.unidata.ucar.edu/mailing_lists/
>
>
>
Ticket Details
===================
Ticket ID: GQN-392793
Department: Support IDV
Priority: High
Status: Closed