Changeset 7c7e131


Ignore:
Timestamp:
Aug 22, 2012 5:40:47 PM (8 years ago)
Author:
zzz <zzz@…>
Branches:
master
Children:
1d41c2fd
Parents:
85fbbf8
Message:
  • SimpleTimer2: Synchronization improvements (ticket #653)
File:
1 edited

Legend:

Unmodified
Added
Removed
  • core/java/src/net/i2p/util/SimpleTimer2.java

    r85fbbf8 r7c7e131  
    3939    private final ScheduledThreadPoolExecutor _executor;
    4040    private final String _name;
    41     private int _count;
     41    private volatile int _count;
    4242    private final int _threads;
    4343
     
    6565    protected SimpleTimer2(I2PAppContext context, String name, boolean prestartAllThreads) {
    6666        _name = name;
    67         _count = 0;
    6867        long maxMemory = Runtime.getRuntime().maxMemory();
    6968        if (maxMemory == Long.MAX_VALUE)
     
    128127    }
    129128
     129    /**
     130     * state of a given TimedEvent
     131     *
     132     * valid transitions:
     133     * {IDLE,CANCELLED,RUNNING} -> SCHEDULED [ -> SCHEDULED ]* -> RUNNING -> {IDLE,CANCELLED,SCHEDULED}
     134     * {IDLE,CANCELLED,RUNNING} -> SCHEDULED [ -> SCHEDULED ]* -> CANCELLED
     135     *
     136     * anything else is invalid.
     137     */
     138    private enum TimedEventState {
     139        IDLE,
     140        SCHEDULED,
     141        RUNNING,
     142        CANCELLED
     143    };
     144   
    130145    /**
    131146     * Similar to SimpleTimer.TimedEvent but users must extend instead of implement,
     
    153168    public static abstract class TimedEvent implements Runnable {
    154169        private final Log _log;
    155         private SimpleTimer2 _pool;
     170        private final SimpleTimer2 _pool;
    156171        private int _fuzz;
    157172        protected static final int DEFAULT_FUZZ = 3;
     
    159174                                         // ... and I expect cancelling this way is more efficient
    160175
     176        /** state of the current event.  All access should be under lock. */
     177        private TimedEventState _state;
     178        /** absolute time this event should run next time. LOCKING: this */
     179        private long _nextRun;
     180        /** whether this was scheduled during RUNNING state.  LOCKING: this */
     181        private boolean _rescheduleAfterRun;
     182       
    161183        /** must call schedule() later */
    162184        public TimedEvent(SimpleTimer2 pool) {
     
    164186            _fuzz = DEFAULT_FUZZ;
    165187            _log = I2PAppContext.getGlobalContext().logManager().getLog(SimpleTimer2.class);
     188            _state = TimedEventState.IDLE;
    166189        }
    167190
     
    178201         * Default 3 ms.
    179202         */
    180         public void setFuzz(int fuzz) {
     203        public synchronized void setFuzz(int fuzz) {
    181204            _fuzz = fuzz;
    182205        }
     
    190213        public synchronized void schedule(long timeoutMs) {
    191214            if (_log.shouldLog(Log.DEBUG))
    192                 _log.debug("Scheduling: " + this + " timeout = " + timeoutMs);
     215                _log.debug("Scheduling: " + this + " timeout = " + timeoutMs + " state: " + _state);
    193216            if (timeoutMs <= 0 && _log.shouldLog(Log.WARN))
    194217                timeoutMs = 1; // otherwise we may execute before _future is updated, which is fine
    195218                               // except it triggers 'early execution' warning logging
    196             _future = _pool.schedule(this, timeoutMs);
     219
     220            // always set absolute time of execution
     221            _nextRun = timeoutMs + System.currentTimeMillis();
     222           
     223            switch(_state) {
     224              case RUNNING:
     225                _rescheduleAfterRun = true;  // signal that we need rescheduling.
     226                break;
     227              case IDLE:  // fall through
     228              case CANCELLED:
     229                _future = _pool.schedule(this, timeoutMs);
     230                _state = TimedEventState.SCHEDULED;
     231                break;
     232              case SCHEDULED: // nothing
     233            }
     234           
    197235        }
    198236
     
    216254         */
    217255        public synchronized void reschedule(long timeoutMs, boolean useEarliestTime) {
     256            final long now = System.currentTimeMillis();
    218257            long oldTimeout;
    219             boolean scheduled = _future != null && !_future.isDone();
     258            boolean scheduled = _state == TimedEventState.SCHEDULED;
    220259            if (scheduled)
    221                 oldTimeout = _future.getDelay(TimeUnit.MILLISECONDS);
     260                oldTimeout = _nextRun - now;
    222261            else
    223262                oldTimeout = timeoutMs;
     263           
    224264            // don't bother rescheduling if within _fuzz ms
    225265            if ((oldTimeout - _fuzz > timeoutMs && useEarliestTime) ||
    226266                (oldTimeout + _fuzz < timeoutMs && !useEarliestTime)||
    227267                (!scheduled)) {
    228                 if (scheduled) {
     268                if (scheduled && (now + timeoutMs) < _nextRun) {
    229269                    if (_log.shouldLog(Log.INFO))
    230                         _log.info("Re-scheduling: " + this + " timeout = " + timeoutMs + " old timeout was " + oldTimeout);
     270                        _log.info("Re-scheduling: " + this + " timeout = " + timeoutMs + " old timeout was " + oldTimeout + " state: " + _state);
    231271                    cancel();
    232272                }
     
    246286        /** returns true if cancelled */
    247287        public synchronized boolean cancel() {
    248             if (_future == null)
    249                 return false;
    250             return _future.cancel(false);
     288            // always clear
     289            _rescheduleAfterRun = false;
     290           
     291            switch(_state) {
     292              case CANCELLED:  // fall through
     293              case IDLE:
     294                break; // my preference is to throw IllegalState here, but let it be.
     295              case RUNNING:    // fall through
     296              case SCHEDULED:
     297                boolean cancelled = _future.cancel(false);
     298                if (cancelled)
     299                    _state = TimedEventState.CANCELLED;
     300                else {} // log something as this could be serious, we remain RUNNING otherwise
     301                return cancelled;
     302            }
     303            return false;
     304           
    251305        }
    252306
     
    256310            long before = System.currentTimeMillis();
    257311            long delay = 0;
     312            synchronized(this) {
     313                if (_rescheduleAfterRun)
     314                    throw new IllegalStateException("rescheduleAfterRun cannot be true here");
     315               
     316                switch(_state) {
     317                  case CANCELLED:
     318                    return; // goodbye
     319                  case IDLE:  // fall through
     320                  case RUNNING:
     321                    throw new IllegalStateException("not possible to be in " + _state);
     322                  case SCHEDULED: // proceed, switch to IDLE in case I need to reschedule
     323                    _state = TimedEventState.IDLE;
     324                }
     325                                               
     326                // if I was rescheduled by the user, re-submit myself to the executor.
     327                int difference = (int)(_nextRun - before); // careful with long uptimes
     328                if (difference > _fuzz) {
     329                    schedule(difference);
     330                    return;
     331                }
     332               
     333                // else proceed to run
     334                _state = TimedEventState.RUNNING;
     335            }
     336            // cancel()-ing after this point only works if the event supports it explicitly
     337            // none of these _future checks should be necessary anymore
    258338            if (_future != null)
    259339                delay = _future.getDelay(TimeUnit.MILLISECONDS);
     
    269349            } catch (Throwable t) {
    270350                _log.log(Log.CRIT, _pool + ": Timed task " + this + " exited unexpectedly, please report", t);
     351            } finally { // must be in finally
     352                synchronized(this) {
     353                    switch(_state) {
     354                      case SCHEDULED:  // fall through
     355                      case IDLE:
     356                        throw new IllegalStateException("can't be " + _state);
     357                      case CANCELLED:
     358                        break; // nothing
     359                      case RUNNING:
     360                        _state = TimedEventState.IDLE;
     361                        // do we need to reschedule?
     362                        if (_rescheduleAfterRun) {
     363                            _rescheduleAfterRun = false;
     364                            schedule(_nextRun - System.currentTimeMillis());
     365                        }
     366                    }
     367                }
    271368            }
    272369            long time = System.currentTimeMillis() - before;
Note: See TracChangeset for help on using the changeset viewer.