400392 - necko unit test test_reopen.js fails randomly on qm-centos5-01

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[(not currently active) Ted Mielczarek]

The test_reopen.js unit test fails randomly on qm-centos5-01:

../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js: FAIL
../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js.log:
>>>>>>>
*** test pending
*** test pending
*** test finished
*** running event loop
*** exiting
*** CHECK FAILED: 2152398863 == 2152398921
JS frame :: /builds/slave/trunk_centos5/mozilla/tools/test-harness/xpcshell-simple/head.js :: do_throw :: line 99
JS frame :: /builds/slave/trunk_centos5/mozilla/tools/test-harness/xpcshell-simple/head.js :: do_check_eq :: line 114
JS frame :: ../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js :: check_throws :: line 43
JS frame :: ../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js :: check_async_open_throws :: line 58
JS frame :: ../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js :: test_channel :: line 95
JS frame :: ../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js :: test_http_channel :: line 111
JS frame :: ../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js :: run_next_test :: line 87
JS frame :: ../../_tests/xpcshell-simple/test_necko/unit/test_reopen.js :: after_channel_closed :: line 83
JS frame :: /builds/slave/trunk_centos5/mozilla/tools/test-harness/xpcshell-simple/head.js :: anonymous :: line 62
JS frame :: /builds/slave/trunk_centos5/mozilla/tools/test-harness/xpcshell-simple/head.js :: anonymous :: line 62
*** FAIL ***

<<<<<<<

This just happened, and it's happened before, seemingly randomly.

Comment 1

[(not currently active) Ted Mielczarek]

FWIW, the test here is failing:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/test/unit/test_reopen.js&rev=1.2#95

It's getting NS_ERROR_IN_PROGRESS when it expects NS_ERROR_ALREADY_OPENED.

Comment 2

[Sylvain Pasche]

Looks like a timing issue:

In the test, the channel is opened, and then reopened async a bit later. The test assumes that mPending will be set to false in the meantime. On my system, this is done by the nsHttpChannel::OnStopRequest which happens before the asyncOpen is reached. I guess that sometimes the OnStopRequest comes after the second asyncOpen call, so that both mPending and mWasOpened are sill true when that second asyncOpen is reached, which makes it return NS_ERROR_IN_PROGRESS.

I propose to check against both NS_ERROR_ALREADY_OPENED or NS_ERROR_IN_PROGRESS there.

Comment 3

[Sylvain Pasche]

Attachment: patch

Comment 4

[Jeff Walden [:Waldo]]

It may be preferable to reset mPending in AsyncOpen in case of errors rather than wait for OnStopRequest to do so; that's probably what I'd do if I were fixing this.

Comment 5

[(not currently active) Ted Mielczarek]

Bumping severity because this confuses people and makes the unit tests less reliable.

Comment 6

[Reed Loden [:reed]]

Comment on attachment 285516 [details] [diff] [review]
patch

Tests don't need approval.

Comment 7

[Reed Loden [:reed]]

Landed tonight before all the big landings tomorrow that might be impacted by this annoying problem.

Checking in netwerk/test/unit/test_reopen.js;
/cvsroot/mozilla/netwerk/test/unit/test_reopen.js,v  <--  test_reopen.js
new revision: 1.3; previous revision: 1.2
done

Thanks for the patch!