sugar freezes when register widget is clicked (if Server is unresponsive)

[lfaraone]

Adapted from ​https://launchpad.net/bugs/617813:

To reproduce the bug follow these steps-

1. Run sugar emulator.
2. set the Jabber server to localhost
3. start "nc -l 8080" in a terminal. (assuming your computer is not running services on 8080)
4. Right click and click on register widget.

This issue occurs when a Jabber server exists but is frozen. That state is reproduced above.

Patch attached.

[sascha_silbe]

If you post the patch to the mailing list (using git-send-email) you'll have a better chance if it getting reviewed soon (it will happen in Trac as well, but might take more time).

Some things I noticed while giving the patch a quick look:

• subject is not really useful
• description is missing
• non-bugfix changes mixed in (what's the reason for the string import change anyway?)
• change descriptions ("New class [...]") belong in the commit description, not the source code
• please consider adding docstrings to the new classes (but only if you come up with something that's more descriptive than the name itself)
• if all we're doing in TimeoutServerProxy is adding a parameter and we only have one trivial caller, why add a new class at all?

+    def make_connection(self, host):
+        host, extra_headers, x509 = self.get_host_info(host)
+        conn = TimeoutHTTP(host, timeout=self.timeout)
+        return conn

Please combine the last two lines. The variable name doesn't add any information as it should be clear from the method name that the return value will be a connection (and if it isn't clear enough, a docstring would be better).

While this the timeout a stopgap to prevent Sugar from hanging indefinitely, a proper fix will additionally need to make the registration asynchronous. It might be too late to ship the full fix in 0.90, though.

[tomeu]

Replying to sascha_silbe:

While this the timeout a stopgap to prevent Sugar from hanging indefinitely, a proper fix will additionally need to make the registration asynchronous. It might be too late to ship the full fix in 0.90, though.

Not sure if this fix is safer than rewriting it to use gio, have been reading about the different hacks that people have used to overcome this xmlrpclib limitation and looks like they depend on non-API stuff that could (and does) get broken between releases.

I think using xmlrpclib.dumps and xmlrpclib.loads together with gio could be simple enough, we are already using gio for async http:

​http://git.sugarlabs.org/projects/sugar/repos/mainline/blobs/master/extensions/cpsection/updater/backends/aslo.py

[dipankar]

The classes that we have added in the code are inter-dependent.
Removal of any of the classes from it will cause fault in program and it will not run.
To chalk out the dependency:

TimeoutServerProxy uses TimeoutTransport.
TimeoutTransport uses TimeoutHTTP.

I tried searching for them in the standard libraries for Python, but couldn't find them.
So, they cannot be removed from the code in any case.

Wish to ask you whether you have an alternative to the above implementation. If yes, please let me know. Thank you.

[sascha_silbe]

Replying to dipankar:

The classes that we have added in the code are inter-dependent.
Removal of any of the classes from it will cause fault in program and it will not run.

What I meant was replacing

    server = TimeoutServerProxy(url)

with

    server = xmlrpclib.ServerProxy(url, transport=TimeoutTransport())

and dropping class TimeoutServerProxy.

[quozl]

Proposed patch ​http://patchwork.sugarlabs.org/patch/260/ has two reviewed-by tags but no acked-by. See also #2432.

[dnarvaez]

Looks like it was landed.