GNOME Bugzilla – Bug 653468
--auto-close broken by fix for #540560
Last modified: 2013-06-04 19:33:02 UTC
The commit that fixed #540560, broke --auto-close. Reverting the commit (ef3a33a142620fee850351d14f4a1c191fc6e273) restores the old behaviour. I think this commit should be reverted in git, and another solution needs to be found for the "corner case" described in #540560 as it broke existing functionality.
Hello, Can you please show us the steps to reproduce the error? I just run here this: for i in {1..10000}; do echo "#Processing item $i"; done | ./zenity --progress --auto-close And it closes as expected when finish processing. What I found was the cancel button didn't close the window before stop process, and this is for both case using --auto-close or not.
I ran into it with a simple "find /tmp | zenity --progress --auto-close" but your testcase gives me a dialog that stays put with: "Processing item 10000", OK button is greyed out.
I just did a "find / | zenity --progress --auto-close" here and it works and close the file when finish. Which version of zenity are you using? Try use the master version. I check with version 3.0.0 from fedora and the git master, and both close after finish...
Just tried with 3.1.5, and the issue is still there. zenity is eating 99% cpu time but the window doesn't close. If I'd attach a gdb session to the process, what info would you be interested in?
Hello, 3.1.5 ? We just release 3.1.2. Yes, please, attach a gdb session, So I can see what's going wrong.
yes, 3.1.5: http://ftp.acc.umu.se/pub/GNOME/sources/zenity/3.1/zenity-3.1.5.tar.xz So attaching a gdb to the process didn't give me any info, so here's the backtrace after sending the hanging process a SIGABRT:
+ Trace 228572
So this appears to be a threading issue...line 80 of uthread_poll.c is this if statement: if (((ret = _thread_sys_poll(fds, numfds, 0)) == 0) && (timeout != 0)) { data.nfds = numfds; data.fds = fds; /* * Clear revents in case of a timeout which leaves fds * unchanged: */ for (n = 0; n < numfds; n++) { fds[n].revents = 0; } curthread->data.poll_data = &data; curthread->interrupted = 0; _thread_kern_sched_state(PS_POLL_WAIT, __FILE__, __LINE__); if (curthread->interrupted) { errno = EINTR; ret = -1; } else { ret = (int)data.nfds; } }
FWIW, this is still happening with zenity-3.4.0.
So I am coming back to this bug because it's a bit annoying for us that auto-close is broken. Using this comparison works for me: if ((condition != G_IO_IN) && (condition = G_IO_HUP)) Successfully tested with both: for i in {1..1000}; do echo "#Processing item $i"; done | zenity --progress for i in {1..10000}; do echo $i; done | zenity --list --column=Number
Created attachment 245864 [details] [review] Detect EOF Correctly HUP is not the correct and portable way to detect EOF.
Please consider the 'Detect EOF Correctly' patch I attached which corrects the problems seen on OpenBSD and should be portable to all platforms. I found this page informative regarding HUP, EOF and poll: http://www.greenend.org.uk/rjk/tech/poll.html Thanks, -Kurt
Arx, I think Kurt finally found the core issue of this. Would is be alright to push it? Thanks.
I will test and if fix the problem, it will be pushed to master untill tomorrow. Right now I am away from computer to test/push. Thanks in advance for the patch, and please, send more. Zenity needs more love!
Two things: 1) I've applied the patch, and I'm not seeing the --auto-close working, perhaps need more changes. 2) If possible, but I don't want to make this mandatory, I'm very happy to see people contributing with Zenity, can you resend the patch in the git diff format?
Hi Arx, thanks for giving it a try. I was able to check this out on Linux and found that we need to keep the watch on G_IO_HUP for it to work on Linux. So if you just disregard the removal of G_IO_HUP from the g_io_add_watch() lines we have a solution that should work for all platforms. The problem is that poll() is not well defined for what to do with EOF, so various platforms do different things. Linux keeps POLLIN set until the last data is read (POLLIN is cleared when the next read would result in 0 byte return (EOF)). Other platforms implement poll() such that POLLIN is set when a read of the file descriptor would not block, so POLLIN is kept set for when read would immediately return 0 (EOF). In other words BSD's keep POLLIN set when a read would result in EOF and Linux doesn't. Thanks, -Kurt
Can we add a macro/ifdef to check if it's BSD or Linux? What do you think?
Looking again at this on Linux and I see that for file based input (i.e. zenity --progress < input.file), poll() keeps POLLIN set even when the next read would result in 0/EOF return. So the changes I made are needed for it to work right when file based input is used on both Linux and OpenBSD. I don't think a macro or ifdef is needed since the changes I submitted work in on both and are needed for Linux when file input is used (< input.file). Just keep the 'G_IO_IN | G_IO_HUP' for g_io_add_watch() calls and we're all good. I can resubmit the patch again if you like with that change. Regards, -Kurt
Created attachment 246013 [details] [review] Corrected EOF Detection For clarity I have updated the diff and have tested this on both Linux and OpenBSD. For Linux it fixes this test case: for i in {1..1000}; do echo "#Processing item $i"; done > input.file zenity --progress --auto-close < input.file For OpenBSD it fixes both pipe input and file based input. Regards, -Kurt
Review of attachment 246013 [details] [review]: Reviewed. Just change the gint status to GIOStatus status
Yes! It works! Thanks for the patch! It's already committed on master. I just change the gint status to GIOStatus status. Your name and email are in the git comments. Keep sending more patches :) https://git.gnome.org/browse/zenity/commit/?id=c89ce9c3812fdc3a2637fd76b42a07385ad50684