Opened 13 years ago
Closed 10 years ago
#2289 closed defect (fixed)
sugar freezes when register widget is clicked (if Server is unresponsive)
Reported by: | lfaraone | Owned by: | tomeu |
---|---|---|---|
Priority: | Unspecified by Maintainer | Milestone: | Unspecified |
Component: | Sugar | Version: | Unspecified |
Severity: | Major | Keywords: | r? |
Cc: | dipankar, sascha, quozl, rgs | Distribution/OS: | Ubuntu |
Bug Status: | Unconfirmed |
Description
Adapted from https://launchpad.net/bugs/617813:
To reproduce the bug follow these steps-
- Run sugar emulator.
- set the Jabber server to localhost
- start "nc -l 8080" in a terminal. (assuming your computer is not running services on 8080)
- 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.
Attachments (2)
Change History (12)
Changed 13 years ago by lfaraone
comment:1 Changed 13 years ago by lfaraone
- Keywords r? added
comment:2 Changed 13 years ago by lfaraone
- Cc dipankar added
comment:3 follow-up: ↓ 4 Changed 13 years ago by sascha_silbe
comment:4 in reply to: ↑ 3 Changed 13 years ago by 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:
comment:5 follow-up: ↓ 6 Changed 13 years ago by dipankar
- Cc sascha added
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.
Changed 13 years ago by dipankar
comment:6 in reply to: ↑ 5 Changed 13 years ago by 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.
comment:7 Changed 13 years ago by quozl
Proposed patch http://patchwork.sugarlabs.org/patch/260/ has two reviewed-by tags but no acked-by. See also #2432.
comment:8 Changed 13 years ago by quozl
- Cc quozl added
comment:9 Changed 13 years ago by rgs
- Cc rgs added
comment:10 Changed 10 years ago by dnarvaez
- Resolution set to fixed
- Status changed from new to closed
Looks like it was landed.
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:
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.