Revert json ops

[ry]

We're experiencing odd benchmarks and errors as a result of the recent ops moving to JSON. They're odd because node and hyper benchmarks are being negatively effected along with deno.

It's disconcerting that the errors below did not cause the tests to fail.

Errors are seen during the benchmarks:

Compile file:///home/travis/build/denoland/deno/tools/deno_tcp.ts
Listening on 127.0.0.1:4544
Listening on 127.0.0.1:4545
error: Uncaught RangeError: start offset of Int32Array should be a multiple of 4
► js/dispatch_minimal.ts:44:17
    at asyncMsgFromRust (js/dispatch_minimal.ts:44:17)
    at asyncMsgFromRust$3 (js/dispatch.ts:57:7)
    at handleAsyncMsgFromRust (shared_queue.js:171:9)
Compile file:///home/travis/build/denoland/deno/js/deps/https/deno.land/std/http/http_bench.ts
http://127.0.0.1:4546/
Listening on http://127.0.0.1:4548
Compile file:///home/travis/build/denoland/deno/tools/deno_http_proxy.ts
Proxy listening on http://127.0.0.1:4547/
error: Uncaught RangeError: start offset of Int32Array should be a multiple of 4
► js/dispatch_minimal.ts:44:17
    at asyncMsgFromRust (js/dispatch_minimal.ts:44:17)
    at asyncMsgFromRust$3 (js/dispatch.ts:57:7)
    at handleAsyncMsgFromRust (shared_queue.js:171:9)
Listening on http://127.0.0.1:4550
Compile file:///home/travis/build/denoland/deno/tools/deno_tcp_proxy.ts
Proxy listening on http://127.0.0.1:4549/
single-thread
http_bench.js start
listening http://127.0.0.1:4544/ rid = 3
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)
unexpected err Connection reset by peer (os error 104)

cc @bartlomieju

[piscisaureus]

Rubberstamp LGTM

[bartlomieju]

@ry @piscisaureus I believe I figured out the cause of the problem, why it wasn't caught in the test and how to solve it.

Cause

The error specified below appears when there's a response for "minimal op" and shared buffer of shared_queue ends up with message that has a length that is not a multiple of 4.

error: Uncaught RangeError: start offset of Int32Array should be a multiple of 4
► js/dispatch_minimal.ts:44:17
    at asyncMsgFromRust (js/dispatch_minimal.ts:44:17)
    at asyncMsgFromRust$3 (js/dispatch.ts:57:7)
    at handleAsyncMsgFromRust (shared_queue.js:171:9)

This situation happens when there's a "json op" that pushes response onto shared queue and is subsequently followed by response of minimal op. That happens because buf from "json op" is not padded ("flatbuffer op" and "minimal op" responses are aligned to 4 bytes).

Tests

The tests did not catch that bug because it requires high concurrency - in unit test suite there are concurrent tests for net module but the concurrency is low (like 2 concurrent accepts). That means that shared_queue can be emptied and reset between response of "json op" and "minimal op" (which causes to reset the offset of shared buffer). That's why the error appears in http benchmark - it's a highly concurrent situation, before last connection is accepted the first connection already reads bytes - this makes it so there's response from "minimal op" and "json op" inside buffer of shared queue (first one is aligned to 4-bytes, while the second might not be).

Possible solution

The easiest solution that comes to mind is to ensure that response buffer from "json op" is aligned to 4-bytes. Additionally I believe that there should be a check in shared_queue.rs::push to ensure that all responses are aligned to 4-bytes.

EDIT: Proof-of-concept solution 7b9d07f

[ry]

@bartlomieju thank you for that analysis - it sounds very likely that you’ve figured it out! We should add a unit test to shared_queue.rs that demos pushing some non-aligned buffers.