public inbox for [email protected]
 help / color / mirror / Atom feed
From: Ammar Faizi <[email protected]>
To: Alviro Iskandar Setiawan <[email protected]>
Cc: Kanna Scarlet <[email protected]>,
	Muhammad Rizki <[email protected]>,
	GNU/Weeb Mailing List <[email protected]>
Subject: [GIT PULL] ncns update (chunked request body support)
Date: Thu, 25 Aug 2022 16:40:47 +0700	[thread overview]
Message-ID: <[email protected]> (raw)

Hi Alviro,

Sorry for sending you a pull request directly this time. I am in a rush
and don't have much time to get this sorted. I spent my day debugging
this. So in a non-chunked request, the body is fully written before the
request even goes in a connected state. So the calling sequence for a
non-chunked request is:

-> CreateNet()
  -> SetURL()
   -> SetRequestHeader()
    -> SetPayload() # request body goes here
     -> Start() in SQE
     --- cr on connected
     --- cr on response started
      -> Read() in SQE
     --- cr on read completed

But in a chunked request, the body should be fully written before we
wait for "on response started" from the Chromium net event. This is
because HTTP works that way. The request should go first before the
client waits for the response. The calling sequence that made the SQE
stuck was like this:

-> CreateNet()
  -> SetURL()
   -> SetRequestHeader()
    -> StartChunkedBody()
     -> Start() in SQE
     --- cr on connected
     --- the ring waits for "on response started"
       -> Write()
       -> Write()
       -> Write()
       -> Write()
       -> Write()
       -> Read() in SQE
       -> Write()
       -> Write()
     --- wait for "on read completed"
     ^^^ stuck here

That calling sequence is wrong, because the read will never complete
before the data stream class in the Chromium reaches its EOF (IOW, the
SetIsFinalChunk() [1] is called). That being said, implies "wait for 'on
read completed'" must only be done after the write operation has finished.

I guess between Friday and Saturday I can ship this feature properly.

Please pull!

[1]: https://source.chromium.org/chromium/chromium/src/+/109fd67d081a16dc8564bd15c146a78f4e068559:net/base/upload_data_stream.h;l=119-121

The following changes since commit 7914975d89b6cb947de27b17df13b2f4830197a1:

   Merge branch 'dev.ring_cleanup' (2022-08-18 05:48:50 +0700)

are available in the Git repository at:

   https://gitlab.torproject.org/ammarfaizi2/ncns.git tags/chnet_body_chunked.2022-08-25

for you to fetch changes up to 77bbcc903899016f3e99ce499ded2056cea77a03:

   chnet: Completely refactor again (2022-08-25 15:51:30 +0700)

----------------------------------------------------------------
chnet_body_chunked.2022-08-25

----------------------------------------------------------------
Ammar Faizi (23):
     chnet: Add initial request body support
     chnet: node: Add set_user_data support on SQE
     tests/js/ring: Update the unit test to utilize set_user_data
     binding.gyp: Add `-ggdb3` flag for better debugging experience
     binding.gyp: Add `-Wno-enum-constexpr-conversion` flag
     chnet: node: Add set_method function to set HTTP method
     chnet: node: Add get_error function to return the error string
     chnet: node: Add set_payload function to set HTTP req body
     tests/js/ring: Add simple HTTP POST request example in NodeJS
     chnet: Split construct URL req creation into a new function
     chnet: Add set request header support
     chnet: node: Fix unused variable warning
     chnet: node: Add set request header function in NodeJS
     tests/js/ring: Add more set header function test
     chnet: node: Don't use static counter for data ID
     tests/js/ring: Add JavaScript class wrapper example
     chnet: Initial chunked request body support
     chnet: Rework the chunked request body interface
     chnet: ring: Refactor the ring completely
     chnet: Use busy-waiting for signal waiter
     chnet: ring: Bump max_entry to 2G
     tests/cpp: Delete basic.cpp as it's no longer relevant
     chnet: Completely refactor again

  binding.gyp         |   4 +-
  chnet/WorkQueue.cc  |  17 +-
  chnet/WorkQueue.h   |   5 +
  chnet/chnet.cc      | 239 +++++++++++++----
  chnet/chnet.h       | 215 +++++++++++++--
  chnet/chnet.old.cc  | 582 ++++++++++++++++++++++++++++++++++++++++
  chnet/chnet.old.h   | 236 ++++++++++++++++
  chnet/chnet_node.cc | 564 +++++++++++++++++++++++++++++---------
  chnet/chnet_node.h  |   6 -
  chnet/chnet_ring.cc | 493 ++++++++++++++++------------------
  chnet/chnet_ring.h  | 200 ++++++++------
  chnet/common.h      |  20 +-
  tests/cpp/basic.cc  |  97 -------
  tests/cpp/ring.cc   | 536 +++++++++++++++++++++++++++++-------
  tests/index.php     |  14 +
  tests/js/ring.js    | 405 ++++++++++++++++++++++++----
  16 files changed, 2797 insertions(+), 836 deletions(-)
  create mode 100644 chnet/chnet.old.cc
  create mode 100644 chnet/chnet.old.h
  delete mode 100644 tests/cpp/basic.cc

-- 
Ammar Faizi

             reply	other threads:[~2022-08-25  9:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-25  9:40 Ammar Faizi [this message]
2022-08-25  9:57 ` [GIT PULL] ncns update (chunked request body support) Alviro Iskandar Setiawan
2022-08-25 18:36   ` Ammar Faizi
2022-08-26  1:34     ` Alviro Iskandar Setiawan
2022-08-26  1:39       ` Alviro Iskandar Setiawan
2022-08-26  1:53         ` Ammar Faizi
2022-08-26  6:24           ` Ammar Faizi
2022-08-26  8:03             ` Ammar Faizi
2022-08-26  8:07               ` Ammar Faizi
2022-08-26 10:37                 ` Alviro Iskandar Setiawan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox