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 664627 - /gapplication/basic test intermittently fails: cmdline re-ordered to before open
/gapplication/basic test intermittently fails: cmdline re-ordered to before open
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gapplication
2.31.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 669303 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-11-23 10:58 UTC by Simon McVittie
Modified: 2012-11-29 23:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix race condition in gapplication/basic test (1.83 KB, patch)
2012-11-29 17:03 UTC, Emilio Pozuelo Monfort
committed Details | Review

Description Simon McVittie 2011-11-23 10:58:42 UTC
When running under `make check` I intermittently (20% of runs?) get this failure:

  /gapplication/basic:                                                 
** ERROR **: 
Expected
-----
activated
open file:///a file:///b
cmdline '40 +' '2'
exit status: 0
-----
Got (full)
-----
activated
cmdline '40 +' '2'
open file:///a file:///b
exit status: 0
-----

I haven't been able to reproduce this by running only ./gapplication; perhaps it's somehow influenced by previous gio tests?
Comment 1 Simon McVittie 2011-11-23 10:59:39 UTC
(In reply to comment #0)
> I haven't been able to reproduce this by running only ./gapplication

... including 50 runs in a loop.

This is on master, commit 1b01109377a (although I've seen it in earlier commits too).
Comment 2 Matthias Clasen 2011-11-23 12:11:43 UTC
The test has

  /* make sure the commandline arrives after the files */
  g_usleep (100000);

I guess this does not work reliably under load, and you manage to have the commandline arrive at the same time as the files.
Comment 3 Allison Karlitskaya (desrt) 2011-11-23 16:51:10 UTC
bah!  no surprise I got bitten by something like this -- I should have known better.
Comment 4 Allison Karlitskaya (desrt) 2012-02-03 14:45:27 UTC
*** Bug 669303 has been marked as a duplicate of this bug. ***
Comment 5 Martin Pitt 2012-06-27 09:19:02 UTC
Confirmed with 2.33.1 on several Debian architectures, e. g.

https://buildd.debian.org/status/fetch.php?pkg=glib2.0&arch=ia64&ver=2.33.3-1&stamp=1340740701

It happens pretty randomly indeed.
Comment 6 Emilio Pozuelo Monfort 2012-11-29 17:03:03 UTC
Created attachment 230214 [details] [review]
Fix race condition in gapplication/basic test

I first thought we could execute the second and third processes synchronously. However, that wouldn't fix the case were the first process is too slow and doesn't become the master.

This other solution is much simpler and fixes all the cases. Since we need to execute all three children in order, we can wait for them to write something to stdout.

The patch works for me, running the gapplication test in a loop for 70 times under heavy load.
Comment 7 Allison Karlitskaya (desrt) 2012-11-29 23:04:03 UTC
Review of attachment 230214 [details] [review]:

Looks good.  Thanks!

::: gio/tests/gapplication.c
@@ +91,3 @@
+   */
+  fd.fd = data->stdout_pipe;
+  fd.events = G_IO_IN | G_IO_HUP | G_IO_ERR;

I think just G_IO_IN here is good enough since poll implicitly adds the other cases... but it's fine either way.
Comment 8 Emilio Pozuelo Monfort 2012-11-29 23:10:36 UTC
(In reply to comment #7)
> Review of attachment 230214 [details] [review]:
> +  fd.fd = data->stdout_pipe;
> +  fd.events = G_IO_IN | G_IO_HUP | G_IO_ERR;
> 
> I think just G_IO_IN here is good enough since poll implicitly adds the other
> cases... but it's fine either way.

I didn't know that... the g_poll docs say to add all three though :p

Committed to master, thanks for the quick review.