Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RolloverFileOutputStream: IllegalStateException Task already scheduled #1469

Closed
joakime opened this issue Apr 13, 2017 · 4 comments

Comments

@joakime
Copy link
Member

commented Apr 13, 2017

Report at 1e46576

BTW, scheduleNextRollover() is called from within RollTask.run()
so, __rollover.schedule(_rollTask, ***) leads to an exception
Exception in thread "org.eclipse.jetty.util.RolloverFileOutputStream" java.lang.IllegalStateException: Task already scheduled or cancelled
at java.util.Timer.sched(Timer.java:401)
at java.util.Timer.schedule(Timer.java:193)
at org.eclipse.jetty.util.RolloverFileOutputStream.scheduleNextRollover(RolloverFileOutputStream.java:216)
at org.eclipse.jetty.util.RolloverFileOutputStream.access$200(RolloverFileOutputStream.java:48)
at org.eclipse.jetty.util.RolloverFileOutputStream$RollTask.run(RolloverFileOutputStream.java:359)
at java.util.TimerThread.mainLoop(Timer.java:555)
at java.util.TimerThread.run(Timer.java:505)
As result, first midnight switches OutputStream out, throws this exception and stops scheduling _rollTask.
Suppose
_rollTask = new RollTask();
before
__rollover.schedule(_rollTask...
will fix that problem.

@joakime joakime added the Bug label Apr 13, 2017

@joakime joakime self-assigned this Apr 13, 2017

joakime referenced this issue Apr 13, 2017
@joakime

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2017

Simplest demonstration of problem.

package time;

import java.util.Timer;
import java.util.concurrent.CountDownLatch;

public class TimerToy
{
    public static void main(String[] args)
    {
        new TimerToy().run();
    }
    
    public class Task extends java.util.TimerTask
    {
        @Override
        public void run()
        {
            try
            {
                System.err.print('.');
                TimerToy.this.scheduleNext(); // can't schedule next from here
                TimerToy.this.latch.countDown();
            }
            catch (Throwable t)
            {
                t.printStackTrace(System.err);
            }
        }
    }
    
    private Timer timer;
    private Task task;
    private CountDownLatch latch;
    
    public TimerToy()
    {
        latch = new CountDownLatch(40);
        task = new Task();
    }
    
    private void scheduleNext()
    {
        timer.schedule(task, 500);
    }
    
    private void run()
    {
        try
        {
            timer = new Timer();
            timer.schedule(task, 500);
            latch.await();
        }
        catch (Throwable t)
        {
            t.printStackTrace(System.err);
        }
    }
}
@joakime

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2017

We cannot go back to periodic Timer usage, as the daylights savings time behavior breaks it for long running Jetty instances.

However, we could use a ScheduledExecutorService instead, like this ...

package time;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

public class TimerToyNG implements Runnable
{
    public static void main(String[] args) throws InterruptedException
    {
        int count = 10;
        TimerToyNG toy = new TimerToyNG(count);
        toy.scheduleNext();
        System.err.println("Waiting completion of " + count + " events");
        toy.latch.await();
        System.err.println("Done");
    }
    
    private ScheduledExecutorService timer;
    private CountDownLatch latch;
    private long lastEvent;
    
    public TimerToyNG(int count)
    {
        timer = Executors.newSingleThreadScheduledExecutor();
        latch = new CountDownLatch(count);
    }
    
    private void scheduleNext()
    {
        lastEvent = System.currentTimeMillis();
        timer.schedule(this, 500, TimeUnit.MILLISECONDS);
    }
    
    @Override
    public void run()
    {
        try
        {
            long now = System.currentTimeMillis();
            System.err.println(" -> " + now + " (" + (now - lastEvent) + ")");
            latch.countDown();
            if (latch.getCount() > 0)
                scheduleNext();
            else
                timer.shutdownNow(); // <-- ew HACK!
        }
        catch (Throwable t)
        {
            t.printStackTrace(System.err);
        }
    }
}

The biggest problem is the running scheduler that never shuts down, preventing the JVM from exiting.

@joakime

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2017

Looks like I can instantiate the ScheduledThreadExecutor with a custom ThreadFactory to set the daemon flag on the threads to get around the shutdown requirement.

        timer = Executors.newSingleThreadScheduledExecutor(r ->
        {
            Thread t = Executors.defaultThreadFactory().newThread(r);
            t.setDaemon(true);
            return t;
        });
@joakime

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2017

Going with a simple new RollTask() technique for now.
Will do the Timer -> Scheduler changeover in a different issue at a later date.

joakime added a commit that referenced this issue Apr 13, 2017

@joakime joakime closed this Apr 13, 2017

nextl00p pushed a commit to i2p/i2p.i2p that referenced this issue Apr 29, 2017

@joakime joakime changed the title IllegalStateException in RolloverFileOutputStream RolloverFileOutputStream: IllegalStateException Task already scheduled Aug 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.