Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition in test suite (0.05) #2

Closed
eserte opened this issue Jul 1, 2016 · 4 comments

Comments

@eserte
Copy link

@eserte eserte commented Jul 1, 2016

0.05 now passes on my linux smokers, but still fails on the freebsd ones. Looking at the truss log while running t/cgi-test-auth-fail.t it seems that there's a race condition:

64932: 0.115133907 0.000014527 open("/dev/stdout",O_WRONLY|O_APPEND|O_CREAT,0666) = 3 (0x3)
...
64932: 0.115357399 0.000010057 open("/dev/stdout",O_WRONLY|O_APPEND|O_CREAT,0666) = 4 (0x4)
...
64932: 0.130372156 0.000086324 write(4,"Date: Fri Jul  1 23:09:19 2016\nRemote IP: localhost (127.0.0.1)\n$VAR1 = {\n          'payload' => 'none'\n        };\n$VAR2 = '<no-x-hub-signature>';\n$VAR3 = 'sha1=78f444a9b7282e9e1ae0f520044e8d9b52243ec9';\n",204) = 204 (0xcc)
...
64932: 0.130554861 0.000015365 write(3,"Authentication failed\n",22) = 22 (0x16)
...
64932: 0.134588335 0.000006146 write(1,"Content-Type: text/plain; charset=utf-8\r\n\r\nAuthentication failed\n",65) = 0 (0x0)

So the header gets written after the body. In fact, if log in t/cgi/cgitest.pl is set to /dev/stderr instead of /dev/stdout, then the test passes (although now the debugging output is visible in the test suite).

@xtaran

This comment has been minimized.

Copy link
Owner

@xtaran xtaran commented Jul 1, 2016

Thanks for the detailed analysis. Will have a look.

@xtaran

This comment has been minimized.

Copy link
Owner

@xtaran xtaran commented Jul 1, 2016

@eserte: Hrm, do you think that something like local $| = 1; inside CGI::Github::Webhook::run() could help?

@eserte

This comment has been minimized.

Copy link
Author

@eserte eserte commented Jul 2, 2016

local $| = 1 on top of the run() method helps.

@xtaran

This comment has been minimized.

Copy link
Owner

@xtaran xtaran commented Jul 2, 2016

Thanks for testing!

@xtaran xtaran closed this in cd41998 Jul 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.