Closed Bug 463254 Opened 12 years ago Closed 12 years ago

httpd.js fails on mac when running test_CrossSiteXHR.html

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9

People

(Reporter: sicking, Assigned: Waldo)

Details

Attachments

(1 file)

Currently test_CrossSiteXHR.html is disabled on mac. If reenabled it just manages to run a few tests and then the server starts returning "500 Internal Server Error" for all requests. Including normal requests to other test files.

What the test is doing is firing lots (hundreds) of XHR requests on an .sjs file. Somehow this seems to trigger some bug that gets httpd.js into a bad state.

For each individual request it is not 100% reproducible. If I comment out the bulk of the tests I can get it to succeed sometimes. The more tests that are reenabled the less likely it is that we'll do them all without getting into the bad state. It pretty quickly gets to a point where it's simply impossible to pass all tests.
With DEBUG enabled to see the exception, it's reasonably obvious -- the exceptions thrown by the SJS handler here were preventing the SJS file from being closed correctly, and we were hitting the OS X open-file limit.  For extra evidence, note that directories load but don't list any files and that the built-in /trace path handler also works correctly -- only paths doing file access would error or act weird.

This patch is on top of the one in bug 462864, but they touch different pieces of code.  The patch probably only applies on top of my mq, tho.  :-)

The patch also addresses bug 389508 comment 70 in passing; the relevant comment should explain all.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #346552 - Flags: review?(honzab)
Comment on attachment 346552 [details] [diff] [review]
Always close file streams even when request handlers could throw exceptions to abort processing early

Not critical, what about maybeAddHeaders function? I don't see the file close call, is converter stream responsible for closing the file? Thought, broken header file should not be checked-in.

Why exactly the change in head_utils? You just want to prevent break of the test chain when there is an exception?
Attachment #346552 - Flags: review?(honzab) → review+
Comment on attachment 346552 [details] [diff] [review]
Always close file streams even when request handlers could throw exceptions to abort processing early

(In reply to comment #2)
> (From update of attachment 346552 [details] [diff] [review])
> Not critical, what about maybeAddHeaders function? I don't see the file close
> call, is converter stream responsible for closing the file? Thought, broken
> header file should not be checked-in.

Yeah, good point -- big difference here is that we read every line in the file all the way to the end and don't know how far that is, so we always read to the end unless there's a syntax error.  Still should deal with it, tho, since others might be throwing less-vetted files at it on occasion.  It's just a matter of moving the ConverterInputStream creation and QI inside the try, then adding a finally block that closes the stream; it'll be in the final checkin.

> Why exactly the change in head_utils? You just want to prevent break of the
> test chain when there is an exception?

All the do_check_* functions throw an exception, so if one failed you might hang silently, since the pending/finished calls are likely unbalanced at that point.  It's just nicer for debugging tests, that's all, because the test can sometimes run to completion even if it fails a few times along the way -- and then the failure log will be dumped by make check, rather than you having to find and view it manually.

Oh, I noticed I forgot to make the TEST_RUNS variable a reasonably high variable (from making the test work before doing that) -- bumping to somewhere around 230ish causes failures when I run them locally, so using 250 to have a reasonably pretty number.
Landed.

(This is an httpd.js bug most specifically, not a Mochitest bug; I've been putting httpd.js bugs in Core:Testing, hence the move.)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Component: Mochitest → Testing
OS: Windows XP → All
Product: Testing → Core
QA Contact: mochitest → testing
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Yay! Great to have you back :)
Flags: in-testsuite+
Component: Testing → httpd.js
Product: Core → Testing
QA Contact: testing → httpd.js
Target Milestone: mozilla1.9.1b2 → mozilla1.9
Component: httpd.js → General
You need to log in before you can comment on or make changes to this bug.