src: remove static variables from tls_wrap

Remove the error message globals.  More prep work for multi-isolate
support.

Reviewed-By: Fedor Indutny <fedor@indutny.com>
PR-URL: https://github.com/node-forward/node/pull/58

diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
index 103d1e30381ff76349c0d784e6d4ad1e3ac51b50..f39739cd2f511f6a9948caeb0dcab84e2048fb90 100644 (file)
--- a/src/stream_wrap.cc
+++ b/src/stream_wrap.cc
@@ -233,8 +233,10 @@ void StreamWrap::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
 
  done:
   const char* msg = wrap->callbacks()->Error();
-  if (msg != nullptr)
+  if (msg != nullptr) {
     req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg));
+    wrap->callbacks()->ClearError();
+  }
   req_wrap_obj->Set(env->bytes_string(),
                     Integer::NewFromUnsigned(env->isolate(), length));
   args.GetReturnValue().Set(err);
@@ -364,8 +366,10 @@ void StreamWrap::WriteStringImpl(const FunctionCallbackInfo<Value>& args) {
 
  done:
   const char* msg = wrap->callbacks()->Error();
-  if (msg != nullptr)
+  if (msg != nullptr) {
     req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg));
+    wrap->callbacks()->ClearError();
+  }
   req_wrap_obj->Set(env->bytes_string(),
                     Integer::NewFromUnsigned(env->isolate(), data_size));
   args.GetReturnValue().Set(err);
@@ -472,8 +476,10 @@ void StreamWrap::Writev(const FunctionCallbackInfo<Value>& args) {
   req_wrap->object()->Set(env->bytes_string(),
                           Number::New(env->isolate(), bytes));
   const char* msg = wrap->callbacks()->Error();
-  if (msg != nullptr)
+  if (msg != nullptr) {
     req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg));
+    wrap->callbacks()->ClearError();
+  }
 
   if (err) {
     req_wrap->~WriteWrap();
@@ -536,8 +542,10 @@ void StreamWrap::AfterWrite(uv_write_t* req, int status) {
   };
 
   const char* msg = wrap->callbacks()->Error();
-  if (msg != nullptr)
+  if (msg != nullptr) {
     argv[3] = OneByteString(env->isolate(), msg);
+    wrap->callbacks()->ClearError();
+  }
 
   req_wrap->MakeCallback(env->oncomplete_string(), ARRAY_SIZE(argv), argv);
 
@@ -592,11 +600,15 @@ void StreamWrap::AfterShutdown(uv_shutdown_t* req, int status) {
 }
 
 
-const char* StreamWrapCallbacks::Error() {
+const char* StreamWrapCallbacks::Error() const {
   return nullptr;
 }
 
 
+void StreamWrapCallbacks::ClearError() {
+}
+
+
 // NOTE: Call to this function could change both `buf`'s and `count`'s
 // values, shifting their base and decrementing their length. This is
 // required in order to skip the data that was successfully written via

diff --git a/src/stream_wrap.h b/src/stream_wrap.h
index dbb969a06c6fe685a2b9c8036e0cd3b912f2c8c6..26d9890bc9da1f7d96e7581cbd0e7f586b4384c7 100644 (file)
--- a/src/stream_wrap.h
+++ b/src/stream_wrap.h
@@ -74,7 +74,8 @@ class StreamWrapCallbacks {
 
   virtual ~StreamWrapCallbacks() = default;
 
-  virtual const char* Error();
+  virtual const char* Error() const;
+  virtual void ClearError();
 
   virtual int TryWrite(uv_buf_t** bufs, size_t* count);
 

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 41e6c100518ea39f6ba191bf4ee0e73355eeb012..650fa8007855b00d61558baad5afb71888d6de18 100644 (file)
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -53,9 +53,6 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
-size_t TLSCallbacks::error_off_;
-char TLSCallbacks::error_buf_[1024];
-
 
 TLSCallbacks::TLSCallbacks(Environment* env,
                            Kind kind,
@@ -118,6 +115,8 @@ TLSCallbacks::~TLSCallbacks() {
     WriteItem* wi = ContainerOf(&WriteItem::member_, q);
     delete wi;
   }
+
+  ClearError();
 }
 
 
@@ -378,32 +377,6 @@ void TLSCallbacks::EncOutCb(uv_write_t* req, int status) {
 }
 
 
-int TLSCallbacks::PrintErrorsCb(const char* str, size_t len, void* arg) {
-  size_t to_copy = error_off_;
-  size_t avail = sizeof(error_buf_) - error_off_ - 1;
-
-  if (avail > to_copy)
-    to_copy = avail;
-
-  memcpy(error_buf_, str, avail);
-  error_off_ += avail;
-  CHECK_LT(error_off_, sizeof(error_buf_));
-
-  // Zero-terminate
-  error_buf_[error_off_] = '\0';
-
-  return 0;
-}
-
-
-const char* TLSCallbacks::PrintErrors() {
-  error_off_ = 0;
-  ERR_print_errors_cb(PrintErrorsCb, this);
-
-  return error_buf_;
-}
-
-
 Local<Value> TLSCallbacks::GetSSLError(int status, int* err, const char** msg) {
   EscapableHandleScope scope(env()->isolate());
 
@@ -420,16 +393,24 @@ Local<Value> TLSCallbacks::GetSSLError(int status, int* err, const char** msg) {
       {
         CHECK(*err == SSL_ERROR_SSL || *err == SSL_ERROR_SYSCALL);
 
-        const char* buf = PrintErrors();
+        BIO* bio = BIO_new(BIO_s_mem());
+        ERR_print_errors(bio);
+
+        BUF_MEM* mem;
+        BIO_get_mem_ptr(bio, &mem);
 
         Local<String> message =
-            OneByteString(env()->isolate(), buf, strlen(buf));
+            OneByteString(env()->isolate(), mem->data, mem->length);
         Local<Value> exception = Exception::Error(message);
 
         if (msg != nullptr) {
           CHECK_EQ(*msg, nullptr);
+          char* const buf = new char[mem->length + 1];
+          memcpy(buf, mem->data, mem->length);
+          buf[mem->length] = '\0';
           *msg = buf;
         }
+        static_cast<void>(BIO_reset(bio));
 
         return scope.Escape(exception);
       }
@@ -523,7 +504,7 @@ bool TLSCallbacks::ClearIn() {
   if (!arg.IsEmpty()) {
     MakePending();
     if (!InvokeQueued(UV_EPROTO))
-      error_ = nullptr;
+      ClearError();
     clear_in_->Reset();
   }
 
@@ -531,10 +512,14 @@ bool TLSCallbacks::ClearIn() {
 }
 
 
-const char* TLSCallbacks::Error() {
-  const char* ret = error_;
+const char* TLSCallbacks::Error() const {
+  return error_;
+}
+
+
+void TLSCallbacks::ClearError() {
+  delete[] error_;
   error_ = nullptr;
-  return ret;
 }
 
 

diff --git a/src/tls_wrap.h b/src/tls_wrap.h
index f6ccb50ab85fc9fc3b0f95c6b4628e84e074b1ac..d25c2e6f36cd1526225c706b71e1a71ae69f4992 100644 (file)
--- a/src/tls_wrap.h
+++ b/src/tls_wrap.h
@@ -52,7 +52,8 @@ class TLSCallbacks : public crypto::SSLWrap<TLSCallbacks>,
                          v8::Handle<v8::Value> unused,
                          v8::Handle<v8::Context> context);
 
-  const char* Error() override;
+  const char* Error() const override;
+  void ClearError() override;
   int TryWrite(uv_buf_t** bufs, size_t* count) override;
   int DoWrite(WriteWrap* w,
               uv_buf_t* bufs,
@@ -124,12 +125,10 @@ class TLSCallbacks : public crypto::SSLWrap<TLSCallbacks>,
     }
   }
 
+  // If |msg| is not nullptr, caller is responsible for calling `delete[] *msg`.
   v8::Local<v8::Value> GetSSLError(int status, int* err, const char** msg);
-  const char* PrintErrors();
 
-  static int PrintErrorsCb(const char* str, size_t len, void* arg);
   static void OnClientHelloParseEnd(void* arg);
-
   static void Wrap(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Receive(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -168,9 +167,6 @@ class TLSCallbacks : public crypto::SSLWrap<TLSCallbacks>,
 #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
   v8::Persistent<v8::Value> sni_context_;
 #endif  // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
-
-  static size_t error_off_;
-  static char error_buf_[1024];
 };
 
 }  // namespace node