GNU/Weeb Mailing List <[email protected]>
 help / color / mirror / Atom feed
* [GIT PULL] ncns update (chunked request body support)
@ 2022-08-25  9:40 Ammar Faizi
  2022-08-25  9:57 ` Alviro Iskandar Setiawan
  0 siblings, 1 reply; 10+ messages in thread
From: Ammar Faizi @ 2022-08-25  9:40 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-25  9:40 [GIT PULL] ncns update (chunked request body support) Ammar Faizi
@ 2022-08-25  9:57 ` Alviro Iskandar Setiawan
  2022-08-25 18:36   ` Ammar Faizi
  0 siblings, 1 reply; 10+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-08-25  9:57 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On Thu, Aug 25, 2022 at 4:40 PM Ammar Faizi wrote:
> Hi Alviro,
>
> Sorry for sending you a pull request directly this time.

no problem

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

oc, i'll wait for it

> Please pull!

pulled into my next branch for testing only, tq

tq

-- Viro

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-25  9:57 ` Alviro Iskandar Setiawan
@ 2022-08-25 18:36   ` Ammar Faizi
  2022-08-26  1:34     ` Alviro Iskandar Setiawan
  0 siblings, 1 reply; 10+ messages in thread
From: Ammar Faizi @ 2022-08-25 18:36 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On 8/25/22 4:57 PM, Alviro Iskandar Setiawan wrote:
> pulled into my next branch for testing only, tq

I am not sure what is going on here, but it always fails when
the written bytes is more than 16K bytes. The current WIP commit
is here:

https://gitlab.torproject.org/ammarfaizi2/ncns/-/commit/20627519f6e882ab7d2309f6a6b6e451fb19edc4

Can you spot a mistake in this version?

I have been debugging this for more than 5 hours and haven't
been able to figure out what is going wrong. I was going to
say that it's a Chromium bug. But thinking of the Chromium
stability so far, I doubt to claim so.

Please review!

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-25 18:36   ` Ammar Faizi
@ 2022-08-26  1:34     ` Alviro Iskandar Setiawan
  2022-08-26  1:39       ` Alviro Iskandar Setiawan
  0 siblings, 1 reply; 10+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-08-26  1:34 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On Fri, Aug 26, 2022 at 1:36 AM Ammar Faizi wrote:
> On 8/25/22 4:57 PM, Alviro Iskandar Setiawan wrote:
> > pulled into my next branch for testing only, tq
>
> I am not sure what is going on here, but it always fails when
> the written bytes is more than 16K bytes. The current WIP commit
> is here:
>
> https://gitlab.torproject.org/ammarfaizi2/ncns/-/commit/20627519f6e882ab7d2309f6a6b6e451fb19edc4
>
> Can you spot a mistake in this version?
>
> I have been debugging this for more than 5 hours and haven't
> been able to figure out what is going wrong. I was going to
> say that it's a Chromium bug. But thinking of the Chromium
> stability so far, I doubt to claim so.

you're tired, get some rest plz, tq for your hard work

> Please review!

found it, the problem is not in that commit, but the previous one

https://gitlab.torproject.org/ammarfaizi2/ncns/-/commit/d113a1611f2eca1f45fd8dd0fe3a02aa92abeb6e#63c8f17eb62643be824f57c1bed41bbf7c80ed5e_50_127

if you split the buffer in the middle of the queue, then you put the
rest of the splitted buffer into the tail, then it may get skipped
because the EOF mark can be queued earlier. Even if it doesn't hit EOF
it's still wrong because the FIFO order is violated when the condition
(c.len_ <= dst_len_) evaluates to true

tq

-- Viro

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-26  1:34     ` Alviro Iskandar Setiawan
@ 2022-08-26  1:39       ` Alviro Iskandar Setiawan
  2022-08-26  1:53         ` Ammar Faizi
  0 siblings, 1 reply; 10+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-08-26  1:39 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On Fri, Aug 26, 2022 at 8:34 AM Alviro Iskandar Setiawan wrote:
> On Fri, Aug 26, 2022 at 1:36 AM Ammar Faizi wrote:
> > On 8/25/22 4:57 PM, Alviro Iskandar Setiawan wrote:
> > > pulled into my next branch for testing only, tq
> >
> > I am not sure what is going on here, but it always fails when
> > the written bytes is more than 16K bytes. The current WIP commit
> > is here:
> >
> > https://gitlab.torproject.org/ammarfaizi2/ncns/-/commit/20627519f6e882ab7d2309f6a6b6e451fb19edc4
> >
> > Can you spot a mistake in this version?
> >
> > I have been debugging this for more than 5 hours and haven't
> > been able to figure out what is going wrong. I was going to
> > say that it's a Chromium bug. But thinking of the Chromium
> > stability so far, I doubt to claim so.
>
> you're tired, get some rest plz, tq for your hard work
>
> > Please review!
>
> found it, the problem is not in that commit, but the previous one
>
> https://gitlab.torproject.org/ammarfaizi2/ncns/-/commit/d113a1611f2eca1f45fd8dd0fe3a02aa92abeb6e#63c8f17eb62643be824f57c1bed41bbf7c80ed5e_50_127
>
> if you split the buffer in the middle of the queue, then you put the
> rest of the splitted buffer into the tail, then it may get skipped
> because the EOF mark can be queued earlier. Even if it doesn't hit EOF
> it's still wrong because the FIFO order is violated when the condition
> (c.len_ <= dst_len_) evaluates to true

better way you shouldn't pop the queue if you're running out of
destination buffer, you can wait for the next ReadInternal() call and
copy the rest uncopied bytes later time, only the deferring is a bit
tricky

tq

-- Viro

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-26  1:39       ` Alviro Iskandar Setiawan
@ 2022-08-26  1:53         ` Ammar Faizi
  2022-08-26  6:24           ` Ammar Faizi
  0 siblings, 1 reply; 10+ messages in thread
From: Ammar Faizi @ 2022-08-26  1:53 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On 8/26/22 8:39 AM, Alviro Iskandar Setiawan wrote:
>> if you split the buffer in the middle of the queue, then you put the
>> rest of the splitted buffer into the tail, then it may get skipped
>> because the EOF mark can be queued earlier. Even if it doesn't hit EOF
>> it's still wrong because the FIFO order is violated when the condition
>> (c.len_ <= dst_len_) evaluates to true
> better way you shouldn't pop the queue if you're running out of
> destination buffer, you can wait for the next ReadInternal() call and
> copy the rest uncopied bytes later time, only the deferring is a bit
> tricky

Ah well, finally got some light!

I understand now, will rework this part later today.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-26  1:53         ` Ammar Faizi
@ 2022-08-26  6:24           ` Ammar Faizi
  2022-08-26  8:03             ` Ammar Faizi
  0 siblings, 1 reply; 10+ messages in thread
From: Ammar Faizi @ 2022-08-26  6:24 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On 8/26/22 8:53 AM, Ammar Faizi wrote:
> Ah well, finally got some light!
> 
> I understand now, will rework this part later today.

Did some experiment, but still stuck with larger buffer. The small
hot patch attached below.

-- 
Ammar Faizi

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 5072 bytes --]

diff --git a/chnet/chnet.cc b/chnet/chnet.cc
index 093660c..d064d52 100644
--- a/chnet/chnet.cc
+++ b/chnet/chnet.cc
@@ -110,23 +110,24 @@ int CHNetChunkedPayload::FlushBufferQueue(void)
 	int ret = 0;
 
 	while (!buf_queue_.empty()) {
-		struct chunk_vec c;
+		struct chunk_vec &c = buf_queue_.front();
 		size_t copy_size;
-
-		c = buf_queue_.front();
-		buf_queue_.pop();
+		bool did_pop;
 
 		if (unlikely(c.len_ == (size_t)~0)) {
+			buf_queue_.pop();
 			SetIsFinalChunk();
 			dst_ptr_ = nullptr;
 			return ret;
 		}
 
 		if (c.len_ <= dst_len_) {
+			buf_queue_.pop();
 			copy_size = c.len_;
+			did_pop = true;
 		} else {
 			copy_size = dst_len_;
-			EnqueueBuffer(&c.buf_[copy_size], c.len_ - copy_size);
+			did_pop = false;
 		}
 
 		memcpy(dst_ptr_, c.buf_, copy_size);
@@ -134,23 +135,43 @@ int CHNetChunkedPayload::FlushBufferQueue(void)
 		dst_len_ -= copy_size;
 		copied_size_ += copy_size;
 		ret += copy_size;
-		free(c.buf_);
+
+		if (did_pop) {
+			free(c.buf_);
+		} else {
+			c.len_ -= copy_size;
+			memmove(c.buf_, &c.buf_[copy_size], c.len_);
+		}
 
 		if (!dst_len_) {
+
+			if (did_pop) {
+				struct chunk_vec &c = buf_queue_.front();
+
+				if (unlikely(c.len_ == (size_t)~0)) {
+					buf_queue_.pop();
+					printf("test\n");
+					SetIsFinalChunk();
+				}
+			}
+
 			dst_ptr_ = nullptr;
 			break;
 		}
 	}
 
-	if (!ret)
+	if (likely(!IsEOF()))
 		return net::ERR_IO_PENDING;
 
+	printf("read ok = %d\n", ret);
 	return ret;
 }
 
 int CHNetChunkedPayload::WriteBuffer(const char *buf, size_t len)
 {
 	size_t copy_size;
+	static std::atomic<uint32_t> p;
+	uint32_t x = p++;
 
 	if (unlikely(!len))
 		return 0;
@@ -164,7 +185,7 @@ int CHNetChunkedPayload::WriteBuffer(const char *buf, size_t len)
 		 * consume the queued buffers in FIFO order.
 		 */
 		EnqueueBuffer(buf, len);
-		printf("1 waaa = %zu\n", len);
+		printf("%u 1 waaa = %zu\n", x, len);
 		goto out;
 	}
 
@@ -178,17 +199,17 @@ int CHNetChunkedPayload::WriteBuffer(const char *buf, size_t len)
 	if (!dst_ptr_) {
 		InvokeOnReadCompleted(copied_size_);
 		EnqueueBuffer(buf, len);
-		printf("2 waaa = %zu\n", len);
+		printf("%u 2 waaa = %zu\n", x, len);
 		goto out;
 	}
 
 	if (len < dst_len_) {
 		copy_size = len;
-		printf("3 waaa = %zu\n", len);
+		printf("%u 3 waaa = %zu\n", x, len);
 	} else {
 		copy_size = dst_len_;
 		EnqueueBuffer(&buf[copy_size], len - copy_size);
-		printf("4 waaa = %zu\n", len);
+		printf("%u 4 waaa = %zu\n", x, len);
 	}
 
 	memcpy(dst_ptr_, buf, copy_size);
@@ -196,8 +217,10 @@ int CHNetChunkedPayload::WriteBuffer(const char *buf, size_t len)
 	dst_len_ -= copy_size;
 	copied_size_ += copy_size;
 
+	printf("dst_len = %zu\n", dst_len_);
 	if (!dst_len_) {
 		dst_ptr_ = nullptr;
+		printf("x\n");
 		InvokeOnReadCompleted(copied_size_);
 	}
 out:
@@ -225,6 +248,9 @@ int CHNetChunkedPayload::ReadInternal(net::IOBuffer *buf, int buf_len)
 {
 	int ret;
 
+	if (buf_len > 1024)
+		buf_len = 1024;
+
 	buf_lock_.lock();
 	dst_ptr_ = buf->data();
 	dst_len_ = buf_len;
diff --git a/chromium/src b/chromium/src
--- a/chromium/src
+++ b/chromium/src
@@ -1 +1 @@
-Subproject commit f7ad9cd81801ffb398777ca64485d5d1af429558
+Subproject commit f7ad9cd81801ffb398777ca64485d5d1af429558-dirty
diff --git a/tests/cpp/ring.cc b/tests/cpp/ring.cc
index bf135c9..0232ae0 100644
--- a/tests/cpp/ring.cc
+++ b/tests/cpp/ring.cc
@@ -503,7 +503,7 @@ static void test_chnet_ring_simple_post_parallel(bool do_sq_start = false)
 
 static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 {
-	constexpr static const uint32_t nr_body_A = 1000 * 15;
+	constexpr static const uint32_t nr_body_A = 1000 * 30;
 	const char *buf;
 	CNRing ring(1);
 	CNRingSQE *sqe;
@@ -513,6 +513,8 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 	uint32_t i;
 	CHNet *ch;
 
+	setvbuf(stdout, NULL, _IOLBF, 4096);
+
 	ch = new CHNet;
 	ch->SetURL("http://127.0.0.1:8000/index.php?action=print_post&key=data");
 	ch->SetMethod("POST");
@@ -542,11 +544,13 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 	assert(ring.Submit() == 1);
 
 	ch->WriteBody("67890", 5);
-	char q[nr_body_A];
-	memset(q, 'A', sizeof(q));
-	ch->WriteBody(q, sizeof(q));
-	ch->WriteBody(q, sizeof(q));
-	ch->WriteBody(q, sizeof(q));
+	// char q[nr_body_A];
+	// memset(q, 'A', sizeof(q));
+	// ch->WriteBody(q, sizeof(q));
+	// ch->WriteBody(q, sizeof(q));
+	// ch->WriteBody(q, sizeof(q));
+	for (i = 0; i < nr_body_A*3; i++)
+		ch->WriteBody("AAAAAAAAAAAAAAA", 15);
 	ch->StopChunkedBody();
 
 	assert(ring.WaitCQE(1) == 1);
@@ -558,6 +562,7 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 	assert(!strncmp("1234567890", buf, cqe->res_));
 	ring.CQAdvance(1);
 
+	printf("xxxxxxxxx\n");
 	total = 0;
 	while (total < nr_body_A*3) {
 		sqe = ring.GetSQE();
@@ -576,7 +581,7 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 			assert(buf[i] == 'A');
 		ring.CQAdvance(1);
 		total += (uint32_t)cqe->res_;
-		printf("total = %u\n", total);
+		// printf("total = %u\n", total);
 	}
 
 	delete ch;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-26  6:24           ` Ammar Faizi
@ 2022-08-26  8:03             ` Ammar Faizi
  2022-08-26  8:07               ` Ammar Faizi
  0 siblings, 1 reply; 10+ messages in thread
From: Ammar Faizi @ 2022-08-26  8:03 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

On 8/26/22 1:24 PM, Ammar Faizi wrote:
> On 8/26/22 8:53 AM, Ammar Faizi wrote:
>> Ah well, finally got some light!
>>
>> I understand now, will rework this part later today.
> 
> Did some experiment, but still stuck with larger buffer. The small
> hot patch attached below.
Now fixed, the incremental on top of my tree attached below.

-- 
Ammar Faizi

[-- Attachment #2: fix.diff --]
[-- Type: text/x-patch, Size: 6063 bytes --]

diff --git a/chnet/chnet.cc b/chnet/chnet.cc
index 093660c..1431561 100644
--- a/chnet/chnet.cc
+++ b/chnet/chnet.cc
@@ -58,12 +58,11 @@ void CHNetChunkedPayload::ClearPendingQueue(void)
 	__must_hold(&buf_lock_)
 {
 	while (!buf_queue_.empty()) {
-		struct chunk_vec c;
+		struct chunk_vec &c = buf_queue_.front();
 
-		c = buf_queue_.front();
-		buf_queue_.pop();
 		if (c.buf_)
 			free(c.buf_);
+		buf_queue_.pop();
 	}
 }
 
@@ -90,7 +89,6 @@ void CHNetChunkedPayload::InvokeOnReadCompleted(int result)
 				&CHNetChunkedPayload::OnReadCompleted,
 				base::Unretained(this), result));
 	} while (unlikely(!p && ++try_count < 10));
-	printf("InvokeOnReadCompleted() = %d\n", result);
 }
 
 void CHNetChunkedPayload::EnqueueBuffer(const char *buf, size_t len)
@@ -109,24 +107,25 @@ int CHNetChunkedPayload::FlushBufferQueue(void)
 {
 	int ret = 0;
 
+	DCHECK(dst_ptr_);
+
 	while (!buf_queue_.empty()) {
-		struct chunk_vec c;
+		struct chunk_vec &c = buf_queue_.front();
 		size_t copy_size;
-
-		c = buf_queue_.front();
-		buf_queue_.pop();
+		bool should_pop;
 
 		if (unlikely(c.len_ == (size_t)~0)) {
+			buf_queue_.pop();
 			SetIsFinalChunk();
-			dst_ptr_ = nullptr;
 			return ret;
 		}
 
 		if (c.len_ <= dst_len_) {
 			copy_size = c.len_;
+			should_pop = true;
 		} else {
 			copy_size = dst_len_;
-			EnqueueBuffer(&c.buf_[copy_size], c.len_ - copy_size);
+			should_pop = false;
 		}
 
 		memcpy(dst_ptr_, c.buf_, copy_size);
@@ -134,7 +133,14 @@ int CHNetChunkedPayload::FlushBufferQueue(void)
 		dst_len_ -= copy_size;
 		copied_size_ += copy_size;
 		ret += copy_size;
-		free(c.buf_);
+
+		if (should_pop) {
+			free(c.buf_);
+			buf_queue_.pop();
+		} else {
+			c.len_ -= copy_size;
+			memmove(c.buf_, &c.buf_[copy_size], c.len_);
+		}
 
 		if (!dst_len_) {
 			dst_ptr_ = nullptr;
@@ -142,9 +148,6 @@ int CHNetChunkedPayload::FlushBufferQueue(void)
 		}
 	}
 
-	if (!ret)
-		return net::ERR_IO_PENDING;
-
 	return ret;
 }
 
@@ -156,39 +159,24 @@ int CHNetChunkedPayload::WriteBuffer(const char *buf, size_t len)
 		return 0;
 
 	buf_lock_.lock();
-
 	if (!dst_ptr_) {
-		/*
-		 * The request is not ready to consume the buffer,
-		 * queue it, the next ReadInternal() call will
-		 * consume the queued buffers in FIFO order.
-		 */
 		EnqueueBuffer(buf, len);
-		printf("1 waaa = %zu\n", len);
 		goto out;
 	}
 
 	FlushBufferQueue();
 
-	/*
-	 * FlushBufferQueue() may set the @dst_ptr_ to nullptr.
-	 * That is when we are running out of desination buffer
-	 * after flushing the buffer queue.
-	 */
 	if (!dst_ptr_) {
 		InvokeOnReadCompleted(copied_size_);
 		EnqueueBuffer(buf, len);
-		printf("2 waaa = %zu\n", len);
 		goto out;
 	}
 
-	if (len < dst_len_) {
+	if (len <= dst_len_) {
 		copy_size = len;
-		printf("3 waaa = %zu\n", len);
 	} else {
 		copy_size = dst_len_;
 		EnqueueBuffer(&buf[copy_size], len - copy_size);
-		printf("4 waaa = %zu\n", len);
 	}
 
 	memcpy(dst_ptr_, buf, copy_size);
@@ -200,6 +188,7 @@ int CHNetChunkedPayload::WriteBuffer(const char *buf, size_t len)
 		dst_ptr_ = nullptr;
 		InvokeOnReadCompleted(copied_size_);
 	}
+
 out:
 	buf_lock_.unlock();
 	return len;
@@ -215,8 +204,7 @@ void CHNetChunkedPayload::StopWriting(void)
 	buf_queue_.push(c);
 	if (dst_ptr_) {
 		FlushBufferQueue();
-		if (!dst_ptr_)
-			InvokeOnReadCompleted(copied_size_);
+		InvokeOnReadCompleted(copied_size_);
 	}
 	buf_lock_.unlock();
 }
@@ -231,6 +219,10 @@ int CHNetChunkedPayload::ReadInternal(net::IOBuffer *buf, int buf_len)
 	copied_size_ = 0;
 	ret = FlushBufferQueue();
 	buf_lock_.unlock();
+
+	if (ret < buf_len && !IsEOF())
+		return net::ERR_IO_PENDING;
+
 	printf("ReadInternal() = %d\n", ret);
 	return ret;
 }
diff --git a/chromium/src b/chromium/src
--- a/chromium/src
+++ b/chromium/src
@@ -1 +1 @@
-Subproject commit f7ad9cd81801ffb398777ca64485d5d1af429558
+Subproject commit f7ad9cd81801ffb398777ca64485d5d1af429558-dirty
diff --git a/tests/cpp/ring.cc b/tests/cpp/ring.cc
index bf135c9..4e13c4e 100644
--- a/tests/cpp/ring.cc
+++ b/tests/cpp/ring.cc
@@ -503,7 +503,7 @@ static void test_chnet_ring_simple_post_parallel(bool do_sq_start = false)
 
 static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 {
-	constexpr static const uint32_t nr_body_A = 1000 * 15;
+	constexpr static const uint32_t nr_body_A = 1000 * 30;
 	const char *buf;
 	CNRing ring(1);
 	CNRingSQE *sqe;
@@ -513,6 +513,8 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 	uint32_t i;
 	CHNet *ch;
 
+	setvbuf(stdout, NULL, _IOLBF, 4096);
+
 	ch = new CHNet;
 	ch->SetURL("http://127.0.0.1:8000/index.php?action=print_post&key=data");
 	ch->SetMethod("POST");
@@ -542,11 +544,8 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 	assert(ring.Submit() == 1);
 
 	ch->WriteBody("67890", 5);
-	char q[nr_body_A];
-	memset(q, 'A', sizeof(q));
-	ch->WriteBody(q, sizeof(q));
-	ch->WriteBody(q, sizeof(q));
-	ch->WriteBody(q, sizeof(q));
+	for (i = 0; i < nr_body_A*3; i++)
+		ch->WriteBody("A", 1);
 	ch->StopChunkedBody();
 
 	assert(ring.WaitCQE(1) == 1);
@@ -576,7 +575,6 @@ static void test_chnet_ring_chunked_post(bool do_sq_start = false)
 			assert(buf[i] == 'A');
 		ring.CQAdvance(1);
 		total += (uint32_t)cqe->res_;
-		printf("total = %u\n", total);
 	}
 
 	delete ch;
@@ -588,16 +586,15 @@ int main(void)
 
 	test_nop();
 	chnet_global_init();
-	test_chnet_ring_chunked_post(true);
-	// for (i = 0; i < 2; i++) {
-		// test_chnet_ring_read(!!i);
-		// test_chnet_ring_partial_read(!!i);
-		// test_chnet_ring_read_parallel(!!i);
-		// test_chnet_ring_partial_read_parallel(!!i);
-		// test_chnet_ring_simple_post(!!i);
-		// test_chnet_ring_simple_post_parallel(!!i);
-		// test_chnet_ring_chunked_post(!!i);
-	// }
+	for (i = 0; i < 2; i++) {
+		test_chnet_ring_read(!!i);
+		test_chnet_ring_partial_read(!!i);
+		test_chnet_ring_read_parallel(!!i);
+		test_chnet_ring_partial_read_parallel(!!i);
+		test_chnet_ring_simple_post(!!i);
+		test_chnet_ring_simple_post_parallel(!!i);
+		test_chnet_ring_chunked_post(!!i);
+	}
 	chnet_global_stop();
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-26  8:03             ` Ammar Faizi
@ 2022-08-26  8:07               ` Ammar Faizi
  2022-08-26 10:37                 ` Alviro Iskandar Setiawan
  0 siblings, 1 reply; 10+ messages in thread
From: Ammar Faizi @ 2022-08-26  8:07 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On 8/26/22 3:03 PM, Ammar Faizi wrote:
> Now fixed, the incremental on top of my tree attached below.

I will send a proper patch for this soon.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] ncns update (chunked request body support)
  2022-08-26  8:07               ` Ammar Faizi
@ 2022-08-26 10:37                 ` Alviro Iskandar Setiawan
  0 siblings, 0 replies; 10+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-08-26 10:37 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: Kanna Scarlet, Muhammad Rizki, GNU/Weeb Mailing List

On Fri, Aug 26, 2022 at 3:07 PM Ammar Faizi wrote:
> On 8/26/22 3:03 PM, Ammar Faizi wrote:
> > Now fixed, the incremental on top of my tree attached below.
>
> I will send a proper patch for this soon.

awesome

tq

-- Viro

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-08-26 10:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25  9:40 [GIT PULL] ncns update (chunked request body support) Ammar Faizi
2022-08-25  9:57 ` 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox