After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 653468 - --auto-close broken by fix for #540560
--auto-close broken by fix for #540560
Status: RESOLVED FIXED
Product: zenity
Classification: Core
Component: general
3.4.x
Other All
: Normal normal
: ---
Assigned To: Zenity Maintainers
Zenity Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-06-27 08:08 UTC by Jasper Lievisse Adriaanse
Modified: 2013-06-04 19:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Detect EOF Correctly (4.02 KB, patch)
2013-06-02 16:47 UTC, Kurt Miller
none Details | Review
Corrected EOF Detection (3.04 KB, patch)
2013-06-04 15:59 UTC, Kurt Miller
accepted-commit_now Details | Review

Description Jasper Lievisse Adriaanse 2011-06-27 08:08:41 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.
Comment 1 Arx Cruz 2011-06-27 12:21:49 UTC
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.
Comment 2 Jasper Lievisse Adriaanse 2011-06-27 14:47:32 UTC
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.
Comment 3 Arx Cruz 2011-07-26 17:25:38 UTC
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...
Comment 4 Jasper Lievisse Adriaanse 2011-09-05 17:48:39 UTC
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?
Comment 5 Arx Cruz 2011-09-24 23:56:51 UTC
Hello,

3.1.5 ? We just release 3.1.2.

Yes, please, attach a gdb session, So I can see what's going wrong.
Comment 6 Jasper Lievisse Adriaanse 2011-09-25 19:21:13 UTC
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:


  • #0 poll
    from /usr/lib/libc.so.60.1
  • #1 poll
    at /usr/src/lib/libpthread/uthread/uthread_poll.c line 80
  • #2 g_main_context_check
    from /usr/local/lib/libglib-2.0.so.2992.0
  • #3 g_main_loop_run
    from /usr/local/lib/libglib-2.0.so.2992.0
  • #4 gtk_main
    from /usr/local/lib/libgtk-3.so.1.0
  • #5 zenity_progress
    at progress.c line 307
  • #6 main
    at main.c line 84
  • #0 poll
    from /usr/lib/libc.so.60.1
  • #1 poll
    at /usr/src/lib/libpthread/uthread/uthread_poll.c line 80

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;
		}
	}
Comment 7 Antoine Jacoutot 2012-04-04 09:13:03 UTC
FWIW, this is still happening with zenity-3.4.0.
Comment 8 Antoine Jacoutot 2012-04-13 17:40:51 UTC
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
Comment 9 Kurt Miller 2013-06-02 16:47:47 UTC
Created attachment 245864 [details] [review]
Detect EOF Correctly

HUP is not the correct and portable way to detect EOF.
Comment 10 Kurt Miller 2013-06-02 16:55:02 UTC
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
Comment 11 Antoine Jacoutot 2013-06-02 19:25:28 UTC
Arx, I think Kurt finally found the core issue of this. Would is be alright to push it?
Thanks.
Comment 12 Arx Cruz 2013-06-02 21:44:28 UTC
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!
Comment 13 Arx Cruz 2013-06-03 18:30:23 UTC
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?
Comment 14 Kurt Miller 2013-06-03 22:16:08 UTC
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
Comment 15 Arx Cruz 2013-06-03 23:06:37 UTC
Can we add a macro/ifdef to check if it's BSD or Linux? 
What do you think?
Comment 16 Kurt Miller 2013-06-04 15:33:51 UTC
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
Comment 17 Kurt Miller 2013-06-04 15:59:55 UTC
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
Comment 18 Arx Cruz 2013-06-04 19:32:14 UTC
Review of attachment 246013 [details] [review]:

Reviewed. Just change the gint status to GIOStatus status
Comment 19 Arx Cruz 2013-06-04 19:33:02 UTC
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