Last Comment Bug 782649 - Remove old IPC::InputStream
: Remove old IPC::InputStream
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Ben Turner (not reading bugmail, use the needinfo flag!)
:
: [PTO to Dec5] Bill McCloskey (:billm)
Mentors:
Depends on: 781256
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-14 07:29 PDT by Ben Turner (not reading bugmail, use the needinfo flag!)
Modified: 2012-08-23 19:22 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch, v1 (48.35 KB, patch)
2012-08-14 07:29 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
khuey: review+
cjones.bugs: review+
Details | Diff | Splinter Review

Description Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-14 07:29:02 PDT
Created attachment 651761 [details] [diff] [review]
Patch, v1

The old IPC::InputStream uses manual WriteParam/ReadParam and also calls do_CreateInstance on a CID string supplied by the child.

Attached patch adds serialize hooks to the remaining input stream classes and switches PNecko stuff to use them.

r? to khuey for the input stream class changes.
r? to cjones for the ipc/ipdl changes (which we discussed over irc).
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-14 23:45:53 PDT
Comment on attachment 651761 [details] [diff] [review]
Patch, v1

r=me on ipc/ipdl changes
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-08-20 11:25:59 PDT
Comment on attachment 651761 [details] [diff] [review]
Patch, v1

Review of attachment 651761 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/glue/InputStreamUtils.cpp
@@ +103,1 @@
>        return nullptr;

MOZ_NOT_REACHED crashes opt build right?  If so, you don't need the return nullptr anymore.

::: netwerk/base/src/nsBufferedStreams.cpp
@@ +493,5 @@
> +
> +    if (mStream) {
> +        nsCOMPtr<nsIIPCSerializableInputStream> stream =
> +            do_QueryInterface(mStream);
> +        NS_ASSERTION(stream, "Wrapped stream is not serializable!");

Should this be a fatal assertion, since we'll crash at the dereference a few lines below?

::: netwerk/base/src/nsMIMEInputStream.cpp
@@ +302,5 @@
> +
> +    if (mData) {
> +        nsCOMPtr<nsIIPCSerializableInputStream> stream =
> +            do_QueryInterface(mData);
> +        NS_ASSERTION(stream, "Wrapped stream is not serializable!");

This too.
Comment 3 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-22 12:24:03 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2)
> Should this be a fatal assertion, since we'll crash at the dereference a few
> lines below?

I don't want to get into that really. Right now this file only uses NS_ASSERTION, and whether those should be fatal or not is up to others.

https://hg.mozilla.org/integration/mozilla-inbound/rev/41a3cdf92063
Comment 4 Ryan VanderMeulen [:RyanVM] 2012-08-22 14:33:26 PDT
Backed out for Linux xpcshell permaorange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9209d9af04d4

https://tbpl.mozilla.org/php/getParsedLog.php?id=14607998&tree=Mozilla-Inbound

TEST-INFO | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit_ipc/test_post_wrap.js | running test ...
TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit_ipc/test_post_wrap.js | test failed (with xpcshell return code: 0), see following log:
>>>>>>>

TEST-INFO | (xpcshell/head.js) | test 1 pending

parent: TEST-INFO | (xpcshell/head.js) | test 2 pending

parent: TEST-INFO | (xpcshell/head.js) | test 2 finished

parent: TEST-INFO | (xpcshell/head.js) | running event loop
child: CHILD-TEST-STARTED
child: TEST-INFO | (xpcshell/head.js) | test 1 pending

child: TEST-INFO | (xpcshell/head.js) | test 2 pending

child: TEST-INFO | (xpcshell/head.js) | test 2 finished

child: TEST-INFO | (xpcshell/head.js) | running event loop

child: TEST-PASS | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit/test_post.js | [serverHandler : 79] POST == POST

child: TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit/test_post.js | --AaB03x
Content-Disposition: form-data; name="body"

0123456789
--AaB03x
Content-Disposition: form-data; name="files"; filename="test_readline6.txt"
Content-Type: application/octet-stream
Content-Length: 4097

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxE
--AaB03x--
 ==  - See following stack:
child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_throw :: line 451
child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: _do_check_eq :: line 545
child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_check_eq :: line 566
child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit/test_post.js :: serverHandler :: line 91
child: native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 2312
child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1170
child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1618
child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1466
child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1335
child: native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

child: TEST-INFO | (xpcshell/head.js) | exiting test
child: CHILD-TEST-COMPLETED
parent: TEST-INFO | (xpcshell/head.js) | test 1 finished

parent: TEST-INFO | (xpcshell/head.js) | exiting test

parent: TEST-INFO | (xpcshell/head.js) | No (+ 0) checks actually run

child: TEST-PASS | ../unit/head_channels.js | [null : 147] 321 == 321

child: TEST-INFO | (xpcshell/head.js) | test 1 finished

child: TEST-INFO | (xpcshell/head.js) | exiting test
NOTE: child process received `Goodbye', closing down
<<<<<<<
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-08-22 17:24:47 PDT
FYI, looks like Windows had issues too.

https://tbpl.mozilla.org/php/getParsedLog.php?id=14612600&tree=Mozilla-Inbound

TEST-INFO | c:\talos-slave\test\build\xpcshell\tests\netwerk\test\unit_ipc\test_post_wrap.js | running test ...

command timed out: 1200 seconds without output, attempting to kill
SIGKILL failed to kill process
using fake rc=-1
program finished with exit code -1
Comment 6 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-22 21:35:34 PDT
Fixed it, https://hg.mozilla.org/integration/mozilla-inbound/rev/759b5d914905
Comment 7 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-22 21:36:07 PDT
(I forgot to add a case statement for MIMEInputStream to DeserializeInputStream())
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-08-23 19:22:51 PDT
https://hg.mozilla.org/mozilla-central/rev/759b5d914905

Note You need to log in before you can comment on or make changes to this bug.