Out of sync issue in valid application logic

[Jani Laakso]

Replace HelloWorld? with this:

package com.itmill.toolkit.demo;

import com.itmill.toolkit.ui.Button;
import com.itmill.toolkit.ui.Window;
import com.itmill.toolkit.ui.Button.ClickEvent;
import com.itmill.toolkit.ui.Button.ClickListener;

public class HelloWorld extends com.itmill.toolkit.Application {

	Window mainWindow = new Window("MainWindow");

	Button b1 = new Button("Switch to button 2");
	Button b2 = new Button("Switch to button 1");

	int state = 0;

	public void init() {
		setMainWindow(mainWindow);
		mainWindow.addComponent(b1);

		b1.addListener(new ClickListener() {
			public void buttonClick(ClickEvent event) {
				System.err.println("Got event from "
						+ event.getButton().getCaption());
				try {
					Thread.sleep(5000);
				} catch (InterruptedException e) {
					e.printStackTrace();
				}
				mainWindow.removeAllComponents();
				if (event.getButton().getCaption().equals("Switch to button 2")) {
					mainWindow.addComponent(b2);
				} else {
					mainWindow.addComponent(b1);
				}
			}
		});

	}

}

Start application and hit button multiple times within five seconds, now observe results (see attachment).

Trunk 4412 in use. Browser should not affect this, but I used Firefox 3 beta 5.

Here's console logs

Starting Jetty servlet container.
-------------------------------------------------
Starting IT Mill Toolkit in Web Mode.
Running in http://localhost:8888
-------------------------------------------------

2008-05-09 16:05:40.502::INFO:  Logging to STDERR via org.mortbay.log.StdErrLog
2008-05-09 16:05:40.566::INFO:  jetty-6.1.7
2008-05-09 16:05:41.340::INFO:  Started SelectChannelConnector@0.0.0.0:8888
Starting Web Browser.
Got event from Switch to button 2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2
Warning: Ignoring variable change for non-existent component, VAR_PID=PID2

[Jani Laakso]

Perhaps #1633 is related to this too.

[Jani Laakso]

Sorry, code was a bit buggy (still valid though): this one works even better :-)

package com.itmill.toolkit.demo;

import com.itmill.toolkit.ui.Button;
import com.itmill.toolkit.ui.Window;
import com.itmill.toolkit.ui.Button.ClickEvent;

public class HelloWorld extends com.itmill.toolkit.Application implements
		Button.ClickListener {

	Window mainWindow = new Window("MainWindow");

	Button b1 = new Button("Switch to button 2");
	Button b2 = new Button("Switch to button 1");

	int state = 0;

	public void init() {
		setMainWindow(mainWindow);
		mainWindow.addComponent(b1);
		b1.addListener(this);
		b2.addListener(this);
	}

	public void buttonClick(ClickEvent event) {
		System.err.println("Got event from " + event.getButton().getCaption());
		try {
			Thread.sleep(5000);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		mainWindow.removeAllComponents();
		if (event.getButton().getCaption().equals("Switch to button 2")) {
			mainWindow.addComponent(b2);
		} else {
			mainWindow.addComponent(b1);
		}

	}

}

Note: this code works fine if you are "slow" enough, click and wait for 5 seconds => OK, click and wait less than 5 seconds before clicking again => bang..

[Matti Tahvonen]

Hi!

I'm closing this as invalid. As with the A letter from Ajax Toolkit does not block events to be sent when another one is still being handled. In your app click on the button removes it from the application, so we naturally expect the application to be in somehow critical shape. That's why we arise the error. Previously we just dropped these issues, but now we are kind of trying to play safer and lead programmers to to it right.

As with all GUI frameworks, you ought to do your expensive work in a thread and show user a proper indication of the process. Especially if you are about to change the GUI a lot after your event (removing components that user still might touch.

If you dislike using threads for expensive operations, you can vote for #1454.

Below is an example how this kind of thing ought to be programmed

package com.itmill.toolkit.demo;

import com.itmill.toolkit.ui.Button;
import com.itmill.toolkit.ui.ProgressIndicator;
import com.itmill.toolkit.ui.Window;
import com.itmill.toolkit.ui.Button.ClickEvent;

public class HelloWorld extends com.itmill.toolkit.Application implements
        Button.ClickListener {

    Window mainWindow = new Window("MainWindow");

    Button b1 = new Button("Switch to button 2");
    Button b2 = new Button("Switch to button 1");

    int state = 0;

    public void init() {
        setMainWindow(mainWindow);
        mainWindow.addComponent(b1);
        b1.addListener(this);
        b2.addListener(this);
    }

    public void buttonClick(final ClickEvent event) {
        System.err.println("Got event from " + event.getButton().getCaption());

        startProgress();
        Thread t = new Thread() {

            public void run() {
                try {
                    sleep(5000);
                    mainWindow.removeAllComponents();
                    if (event.getButton().getCaption().equals(
                            "Switch to button 2")) {
                        mainWindow.addComponent(b2);
                    } else {
                        mainWindow.addComponent(b1);
                    }
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        };
        t.start();
    }

    public void startProgress() {
        ProgressIndicator pi = new ProgressIndicator();
        pi.setIndeterminate(true);
        pi.setPollingInterval(200);
        mainWindow.removeAllComponents();
        mainWindow.addComponent(pi);
    }

}

[Jani Laakso]

Guys, this is not the case. Think of it again, where do you mark the line, which server-side tasks should be handled with this kind of excessive code: 10ms? 100ms? 300ms? 1000ms?

You can easily reproduce this issue with Feature Browser too. Anyone who tries out our demos from slower latency networks (e.g. US) sees our framework broken!

Also, it is very important to spit out error message (stderr) always when this kind of situation occurs

• if you do report errors then developers may see them and react to issues
• if you do not report errors then developers do not even understand that there is an issue because users typically do not raport "out of synch" issues back to developers (they just feel framework is broken and leave)

Current implementation is good in theory but not in practise, one solution is to get rid of "out of synch" message by default. Perhaps there is a way to better analyze when client-side is actually "out of synch" but I'll leave this for you guys :-)

I strongly suggest we do testing in "ADSL" styled networks, as communication from localhost to localhost is not typical case in any production system. use ipfw or some other nice tool.

See also #1633 (again)

[Marc Englund]

This is not as easy as it seems:

Pretending that variable changes went trough, when they were actually silently ignored, is not good. It works in (and is practical) is some instances, like Janis example above, but unfortunately we can not know if it's safe to ignore a variable change or not (e.g "Stop nuclear reactor" -button).

In fact, I'll go as far as to claim that it's usually bad to silently ignore variable changes. And if this situation occurs, the client and server is really out-of-sync.

You can currently sort of get the old behaviour by saying

    public static SystemMessages getSystemMessages() {
        CustomizedSystemMessages c = new CustomizedSystemMessages();
        c.setOutOfSyncCaption(null);
        c.setOutOfSyncMessage(null);
        return c;
    }

(this will cause a page refresh, though.)

We should think long and hard about ways to do this is a more transparent way, but currently I just don't see how (of course, today is monday ;-)

[Jani Laakso]

I understand how hard issue this is, I am just worried that more inexperienced users / developers see this feature actually as an regression.

I strongly suggest you do #1639 properly for 5.2.0-rcs, before releasing 5.2.0. We have to make sure our own demos do not produce "Out of synch" message at least too "often".

I also suggest that you cut & paste this ticket contents to manual, see 9.1. Special Characteristics of AJAX Applications http://dev.itmill.com/ext/manual/ch09.html#section.application.pages

I insist that you do System.err.println() each time "Out of synch" message is thrown out at the client side.

Cheers!

[Matti Tahvonen]

waiting for comments, solutions from joonas

[Matti Tahvonen]

Idea: As some components like Table do some uncritical lazy updating and possibly cause out of sync problems, we could mark such variable changes an uncritical. If those fail, terminal could simply drop them, but raise error on button clicks etc.

[Joonas Lehtinen]

Decided to have the following resolution with Marc:

1) Keep the new SystemMessages? API proposed by marc as is 2) Keep the out-of-sync error as proposed by marc 3) Add the following functionality to client: a) When an XHR is ongoing, prevent any immediate variablechanges and queue them instead, b) after this HXR has returned, drop any variable changes to non-existing variable-ids from the var-change-queue and submit new queue

Passing the ticket back to marc...

[Matti Tahvonen]

in [4478].

Now ongoing request is first handled before sending queued variable changes.