Scheduler locks main thread?

Since I ended up using the scheduler for updating sign text I ran into a new problem.
Sometimes the call to BukkitScheduler.scheduleSyncDelayedTask() doesn't return and the server stops responding completely. When I added System.out messages before and after the call, the after message wouldn't print. This doesn't happen on every call. Sometime it doesn't happen at all and sometimes it can happen every 4 calls or so.
Is it even possible that the method blocks? Could the synchronize block freeze everything up?
Would appreciate any ideas...

I also tried this and didn't get any output:

Code:

System.out.println(redstoneChips.getServer().getScheduler().scheduleSyncDelayedTask(redstoneChips, signUpdateTask, 1));

so I'm pretty sure the method never returns:

here's the scheduler method source:
http://javadoc.lukegb.com/Bukkit/d0/dff/CraftScheduler_8java_source.html#l00150

EDIT: after adding some debug messages to the scheduler code, i think it happens when there's a new scheduleSyncDelayedTask call while another call is still executing inside the synchronized block.
--- merged: Feb 12, 2011 10:58 PM ---

EDIT2: forgot to mention that the call to scheduleSyncDelayedTask is actually triggered by another scheduler task.

 

Is there any particular reason you need to run tasks in the main server thread? If not, you can use the scheduleAsync* methods (instead of scheduleSync*) to schedule tasks that will run in a separate thread managed by the scheduler.

 

I think the particular reason would be that I'm updating text of signs or lever states in the scheduled tasks. Using async doesn't update the sign at all and can cause the server to crash.

EDIT: I tried the async method and it still locks up.

 

Adding to this - I have an async call that changes blocks after a timeout (of say 10 seconds). For some reason if there is only 1 and sometimes 2 people on the server I have no issue. Once I had 3 people on my server I started seeing this error:

Code:

java.lang.IllegalStateException: TickNextTick list out of synch

        at net.minecraft.server.World.a(World.java:1539)
        at net.minecraft.server.World.f(World.java:1459)
        at net.minecraft.server.WorldServer.f(WorldServer.java:46)
        at net.minecraft.server.MinecraftServer.h(MinecraftServer.java:307)
        at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:232)
        at net.minecraft.server.ThreadServerApplication.run(SourceFile:512)

 

If you're changing blocks on the task then you can't use async, use sync instead and the error will go away.

 

That should be OK.

Do you have the code itself, so I can run/check it.

 

Right now, most API calls are not thread-safe, meaning that only the main thread should call them. Aysnc calls may or may not work, and what you're seeing is the result of that.

I know Bukkit is working on a thread-safe API, but I wish it was coming sooner.

 

I think submitting an async task to submit the sync task would be a workaround for the current implementation.
--- merged: Feb 13, 2011 4:02 AM ---
Bah, the forum ate my post on this.

I have submitted a pull request that fixes this. Sorry about that, I was pretty careful, but guess not careful enough.

An explanation for the deadlock can be found in the pull request comments.

This is my test code, that I used to test the fix. Also, the code is available on GitHub if anyone wants to look at it and see if there are any other potential issues.

Code:

    public class SyncTesting implements Runnable {

        public void run() {

            server.broadcastMessage("about to try to sync task");
            System.out.println("submitting outer task");

            System.out.println( "ID->" + server.getScheduler().scheduleSyncDelayedTask(p, new Runnable() {

                public void run() {

                    System.out.println("Running inner task");
                    server.broadcastMessage("running inner task");
                }

            }, 0));

        }

    }


    public void onPlayerCommand(PlayerChatEvent event) {

        String message = event.getMessage();

        Player player = event.getPlayer();

        String[] split = message.split(" ");

        if( split[0].equals("/syncsync")) {

            for(int cnt=0;cnt<10;cnt++) {

                System.out.println("On command loop Submitting task " + cnt + " id->" + p.getServer().getScheduler().scheduleSyncDelayedTask(p, new SyncTesting(), 0));
            }

        }
    }

 

Looks like your pull request was just built - As my code now uses the Sync versions of the methods, I will keep you updated if I see any issues!

 

Concurrency must be the work of the devil!
Anyways, so far so good . I think the problem is indeed solved. After about 15 mins of testing up constantly updating text on 8 signs I didn't get a lockup even once. Thanks for solving it so quickly and thanks EvilSeph for pushing it in record time...
Signs are still acting out as expected. I'm not sure, but I think that more than 1 sign update or a bit more on a single tick causes the client updates to stop. At least I can be pretty certain my plugin won't crash anybody's server now.

Btw, Raphfrk, before the fix I did use a thread for calling the scheduler (similar to your async suggestion) to prevent the lockup and it solved it for 99% of the time and no thread were left hanging or something like that, but eventually after a few minutes it still deadlocked.

 

@Lologarithm

Do you still see that TickNextTick thing?