public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH ncns v1 0/2] ncns fixes
@ 2022-08-17 13:35 Ammar Faizi
  2022-08-17 13:35 ` [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero Ammar Faizi
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ammar Faizi @ 2022-08-17 13:35 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List

Hi Al,

Yet another small series for ncns.

1) Initialize `read_ret_` to zero.

We can't rely on read_ret_ value being random at initialization. This
caused a real crash where @ret value being random (83382480 in this
case):

  ret = 83382480
  [0817/093957.227555:FATAL:scoped_refptr.h(272)] Check failed: ptr_.
  #0 0x7fa9a8a6b62c <unknown>
  #1 0x7fa9a87cb30a <unknown>
  #2 0x7fa9a87cb2c5 <unknown>
  #3 0x7fa9a881c799 <unknown>
  #4 0x7fa9a881cfa9 <unknown>
  #5 0x7fa9a878537b <unknown>
  #6 0x7fa9a93866b6 <unknown>
  #7 0x7fa9a93121fa <unknown>
  #8 0x7fa9a931debd <unknown>
  #9 0x000000b0deed <unknown>
  #10 0x000000d972d0 <unknown>
  #11 0x000000d9880f <unknown>
  #12 0x0000016dabb9 <unknown>

2) Set error string when the Chromium hits net error.

Translate the error code to string when the Chromium hits a net error.
This makes debugging easier for the callers because now we can know
what the returned error code means, easily.

Signed-off-by: Ammar Faizi <[email protected]>
---
Ammar Faizi (2):
  chnet: Initialize `read_ret_` to zero
  chnet: Set error string when the Chromium hits net error

 chnet/chnet.cc | 12 ++++++++++++
 chnet/chnet.h  |  2 ++
 2 files changed, 14 insertions(+)


base-commit: 971f49f53c730aa82466b251a2b682c0827c3ec4
-- 
Ammar Faizi

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

* [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero
  2022-08-17 13:35 [PATCH ncns v1 0/2] ncns fixes Ammar Faizi
@ 2022-08-17 13:35 ` Ammar Faizi
  2022-08-17 13:40   ` Alviro Iskandar Setiawan
  2022-08-17 13:35 ` [PATCH ncns v1 2/2] chnet: Set error string when the Chromium hits net error Ammar Faizi
  2022-08-17 13:41 ` [PATCH ncns v1 0/2] ncns fixes Alviro Iskandar Setiawan
  2 siblings, 1 reply; 5+ messages in thread
From: Ammar Faizi @ 2022-08-17 13:35 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List

We can't rely on read_ret_ value being random at initialization. This
caused a real crash where @ret value being random (83382480 in this
case):

  ret = 83382480
  [0817/093957.227555:FATAL:scoped_refptr.h(272)] Check failed: ptr_.
  #0 0x7fa9a8a6b62c <unknown>
  #1 0x7fa9a87cb30a <unknown>
  #2 0x7fa9a87cb2c5 <unknown>
  #3 0x7fa9a881c799 <unknown>
  #4 0x7fa9a881cfa9 <unknown>
  #5 0x7fa9a878537b <unknown>
  #6 0x7fa9a93866b6 <unknown>
  #7 0x7fa9a93121fa <unknown>
  #8 0x7fa9a931debd <unknown>
  #9 0x000000b0deed <unknown>
  #10 0x000000d972d0 <unknown>
  #11 0x000000d9880f <unknown>
  #12 0x0000016dabb9 <unknown>

Signed-off-by: Ammar Faizi <[email protected]>
---
 chnet/chnet.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/chnet/chnet.cc b/chnet/chnet.cc
index 37c0b03..c870e05 100644
--- a/chnet/chnet.cc
+++ b/chnet/chnet.cc
@@ -43,6 +43,7 @@ CHNetDelegate::CHNetDelegate(void):
 {
 	base::Thread::Options options(base::MessagePumpType::IO, 0);
 	CHECK(thread_.StartWithOptions(std::move(options)));
+	read_ret_.store(0, std::memory_order_relaxed);
 }
 
 static void CHNetDelegateDestruct(std::unique_ptr<URLRequest> *url_req,
-- 
Ammar Faizi


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

* [PATCH ncns v1 2/2] chnet: Set error string when the Chromium hits net error
  2022-08-17 13:35 [PATCH ncns v1 0/2] ncns fixes Ammar Faizi
  2022-08-17 13:35 ` [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero Ammar Faizi
@ 2022-08-17 13:35 ` Ammar Faizi
  2022-08-17 13:41 ` [PATCH ncns v1 0/2] ncns fixes Alviro Iskandar Setiawan
  2 siblings, 0 replies; 5+ messages in thread
From: Ammar Faizi @ 2022-08-17 13:35 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List

Translate the error code to string when the Chromium hits a net error.
This makes debugging easier for the callers because now we can know
what the returned error code means, easily.

Signed-off-by: Ammar Faizi <[email protected]>
---
 chnet/chnet.cc | 11 +++++++++++
 chnet/chnet.h  |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/chnet/chnet.cc b/chnet/chnet.cc
index c870e05..9d19882 100644
--- a/chnet/chnet.cc
+++ b/chnet/chnet.cc
@@ -149,6 +149,9 @@ int CHNetDelegate::Read(int size)
 
 void CHNetDelegate::OnResponseStarted(URLRequest *url_req, int net_err)
 {
+	if (unlikely(net_err < 0))
+		SetError(net_err);
+
 	if (cb.response_started_)
 		cb.response_started_(cb.response_started_data_, url_req,
 				     net_err);
@@ -156,11 +159,19 @@ void CHNetDelegate::OnResponseStarted(URLRequest *url_req, int net_err)
 
 void CHNetDelegate::OnReadCompleted(URLRequest *url_req, int bytes_read)
 {
+	if (unlikely(bytes_read < 0))
+		SetError(bytes_read);
+
 	read_ret_.store(bytes_read);
 	if (cb.read_completed_)
 		cb.read_completed_(cb.read_completed_data_, url_req, bytes_read);
 }
 
+void CHNetDelegate::SetError(int err_code)
+{
+	err_ = "chromium_net_err:" + ErrorToString(err_code);
+}
+
 } /* namespace net */
 
 CHNet::CHNet(void)
diff --git a/chnet/chnet.h b/chnet/chnet.h
index f91b25b..15ba8cd 100644
--- a/chnet/chnet.h
+++ b/chnet/chnet.h
@@ -26,6 +26,7 @@
 #include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request.h"
+#include "net/base/net_errors.h"
 #define CHNET_EXPORT COMPONENT_EXPORT(CHNET)
 
 #else /* #ifdef __FOR_CHROMIUM_INTERNAL */
@@ -73,6 +74,7 @@ public:
 	int Start(void);
 	int Read(int size);
 	void _Read(Waiter *sig, int size);
+	void SetError(int err_code);
 
 	void OnResponseStarted(URLRequest *url_req, int net_error) override;
 	void OnReadCompleted(URLRequest *url_req, int bytes_read) override;
-- 
Ammar Faizi


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

* Re: [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero
  2022-08-17 13:35 ` [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero Ammar Faizi
@ 2022-08-17 13:40   ` Alviro Iskandar Setiawan
  0 siblings, 0 replies; 5+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-08-17 13:40 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: GNU/Weeb Mailing List

On Wed, Aug 17, 2022 at 8:35 PM Ammar Faizi wrote:
> We can't rely on read_ret_ value being random at initialization. This
> caused a real crash where @ret value being random (83382480 in this
> case):

good catch btw, tq for the fix

tq

-- Viro

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

* Re: [PATCH ncns v1 0/2] ncns fixes
  2022-08-17 13:35 [PATCH ncns v1 0/2] ncns fixes Ammar Faizi
  2022-08-17 13:35 ` [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero Ammar Faizi
  2022-08-17 13:35 ` [PATCH ncns v1 2/2] chnet: Set error string when the Chromium hits net error Ammar Faizi
@ 2022-08-17 13:41 ` Alviro Iskandar Setiawan
  2 siblings, 0 replies; 5+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-08-17 13:41 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: GNU/Weeb Mailing List

On Wed, Aug 17, 2022 at 8:35 PM Ammar Faizi wrote:
> Hi Al,
>
> Yet another small series for ncns.

applied

tq

-- Viro

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

end of thread, other threads:[~2022-08-17 13:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-17 13:35 [PATCH ncns v1 0/2] ncns fixes Ammar Faizi
2022-08-17 13:35 ` [PATCH ncns v1 1/2] chnet: Initialize `read_ret_` to zero Ammar Faizi
2022-08-17 13:40   ` Alviro Iskandar Setiawan
2022-08-17 13:35 ` [PATCH ncns v1 2/2] chnet: Set error string when the Chromium hits net error Ammar Faizi
2022-08-17 13:41 ` [PATCH ncns v1 0/2] ncns fixes 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