Skip to content

Commit 83d2cb6

Browse files
bnoordhuisindutny
bnoordhuis
authored and
indutny
committed
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: node-forward/node#58
1 parent 8ba39b0 commit 83d2cb6

4 files changed

Lines changed: 42 additions & 48 deletions

File tree

‎src/stream_wrap.cc‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,10 @@ void StreamWrap::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
233233

234234
done:
235235
const char* msg = wrap->callbacks()->Error();
236-
if (msg != nullptr)
236+
if (msg != nullptr) {
237237
req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg));
238+
wrap->callbacks()->ClearError();
239+
}
238240
req_wrap_obj->Set(env->bytes_string(),
239241
Integer::NewFromUnsigned(env->isolate(), length));
240242
args.GetReturnValue().Set(err);
@@ -364,8 +366,10 @@ void StreamWrap::WriteStringImpl(const FunctionCallbackInfo<Value>& args) {
364366

365367
done:
366368
const char* msg = wrap->callbacks()->Error();
367-
if (msg != nullptr)
369+
if (msg != nullptr) {
368370
req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg));
371+
wrap->callbacks()->ClearError();
372+
}
369373
req_wrap_obj->Set(env->bytes_string(),
370374
Integer::NewFromUnsigned(env->isolate(), data_size));
371375
args.GetReturnValue().Set(err);
@@ -472,8 +476,10 @@ void StreamWrap::Writev(const FunctionCallbackInfo<Value>& args) {
472476
req_wrap->object()->Set(env->bytes_string(),
473477
Number::New(env->isolate(), bytes));
474478
const char* msg = wrap->callbacks()->Error();
475-
if (msg != nullptr)
479+
if (msg != nullptr) {
476480
req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg));
481+
wrap->callbacks()->ClearError();
482+
}
477483

478484
if (err) {
479485
req_wrap->~WriteWrap();
@@ -536,8 +542,10 @@ void StreamWrap::AfterWrite(uv_write_t* req, int status) {
536542
};
537543

538544
const char* msg = wrap->callbacks()->Error();
539-
if (msg != nullptr)
545+
if (msg != nullptr) {
540546
argv[3] = OneByteString(env->isolate(), msg);
547+
wrap->callbacks()->ClearError();
548+
}
541549

542550
req_wrap->MakeCallback(env->oncomplete_string(), ARRAY_SIZE(argv), argv);
543551

@@ -592,11 +600,15 @@ void StreamWrap::AfterShutdown(uv_shutdown_t* req, int status) {
592600
}
593601

594602

595-
const char* StreamWrapCallbacks::Error() {
603+
const char* StreamWrapCallbacks::Error() const {
596604
return nullptr;
597605
}
598606

599607

608+
void StreamWrapCallbacks::ClearError() {
609+
}
610+
611+
600612
// NOTE: Call to this function could change both `buf`'s and `count`'s
601613
// values, shifting their base and decrementing their length. This is
602614
// required in order to skip the data that was successfully written via

‎src/stream_wrap.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ class StreamWrapCallbacks {
7474

7575
virtual ~StreamWrapCallbacks() = default;
7676

77-
virtual const char* Error();
77+
virtual const char* Error() const;
78+
virtual void ClearError();
7879

7980
virtual int TryWrite(uv_buf_t** bufs, size_t* count);
8081

‎src/tls_wrap.cc‎

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ using v8::Object;
5353
using v8::String;
5454
using v8::Value;
5555

56-
size_t TLSCallbacks::error_off_;
57-
char TLSCallbacks::error_buf_[1024];
58-
5956

6057
TLSCallbacks::TLSCallbacks(Environment* env,
6158
Kind kind,
@@ -118,6 +115,8 @@ TLSCallbacks::~TLSCallbacks() {
118115
WriteItem* wi = ContainerOf(&WriteItem::member_, q);
119116
delete wi;
120117
}
118+
119+
ClearError();
121120
}
122121

123122

@@ -378,32 +377,6 @@ void TLSCallbacks::EncOutCb(uv_write_t* req, int status) {
378377
}
379378

380379

381-
int TLSCallbacks::PrintErrorsCb(const char* str, size_t len, void* arg) {
382-
size_t to_copy = error_off_;
383-
size_t avail = sizeof(error_buf_) - error_off_ - 1;
384-
385-
if (avail > to_copy)
386-
to_copy = avail;
387-
388-
memcpy(error_buf_, str, avail);
389-
error_off_ += avail;
390-
CHECK_LT(error_off_, sizeof(error_buf_));
391-
392-
// Zero-terminate
393-
error_buf_[error_off_] = '\0';
394-
395-
return 0;
396-
}
397-
398-
399-
const char* TLSCallbacks::PrintErrors() {
400-
error_off_ = 0;
401-
ERR_print_errors_cb(PrintErrorsCb, this);
402-
403-
return error_buf_;
404-
}
405-
406-
407380
Local<Value> TLSCallbacks::GetSSLError(int status, int* err, const char** msg) {
408381
EscapableHandleScope scope(env()->isolate());
409382

@@ -420,16 +393,24 @@ Local<Value> TLSCallbacks::GetSSLError(int status, int* err, const char** msg) {
420393
{
421394
CHECK(*err == SSL_ERROR_SSL || *err == SSL_ERROR_SYSCALL);
422395

423-
const char* buf = PrintErrors();
396+
BIO* bio = BIO_new(BIO_s_mem());
397+
ERR_print_errors(bio);
398+
399+
BUF_MEM* mem;
400+
BIO_get_mem_ptr(bio, &mem);
424401

425402
Local<String> message =
426-
OneByteString(env()->isolate(), buf, strlen(buf));
403+
OneByteString(env()->isolate(), mem->data, mem->length);
427404
Local<Value> exception = Exception::Error(message);
428405

429406
if (msg != nullptr) {
430407
CHECK_EQ(*msg, nullptr);
408+
char* const buf = new char[mem->length + 1];
409+
memcpy(buf, mem->data, mem->length);
410+
buf[mem->length] = '\0';
431411
*msg = buf;
432412
}
413+
static_cast<void>(BIO_reset(bio));
433414

434415
return scope.Escape(exception);
435416
}
@@ -523,18 +504,22 @@ bool TLSCallbacks::ClearIn() {
523504
if (!arg.IsEmpty()) {
524505
MakePending();
525506
if (!InvokeQueued(UV_EPROTO))
526-
error_ = nullptr;
507+
ClearError();
527508
clear_in_->Reset();
528509
}
529510

530511
return false;
531512
}
532513

533514

534-
const char* TLSCallbacks::Error() {
535-
const char* ret = error_;
515+
const char* TLSCallbacks::Error() const {
516+
return error_;
517+
}
518+
519+
520+
void TLSCallbacks::ClearError() {
521+
delete[] error_;
536522
error_ = nullptr;
537-
return ret;
538523
}
539524

540525

‎src/tls_wrap.h‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ class TLSCallbacks : public crypto::SSLWrap<TLSCallbacks>,
5252
v8::Handle<v8::Value> unused,
5353
v8::Handle<v8::Context> context);
5454

55-
const char* Error() override;
55+
const char* Error() const override;
56+
void ClearError() override;
5657
int TryWrite(uv_buf_t** bufs, size_t* count) override;
5758
int DoWrite(WriteWrap* w,
5859
uv_buf_t* bufs,
@@ -124,12 +125,10 @@ class TLSCallbacks : public crypto::SSLWrap<TLSCallbacks>,
124125
}
125126
}
126127

128+
// If |msg| is not nullptr, caller is responsible for calling `delete[] *msg`.
127129
v8::Local<v8::Value> GetSSLError(int status, int* err, const char** msg);
128-
const char* PrintErrors();
129130

130-
static int PrintErrorsCb(const char* str, size_t len, void* arg);
131131
static void OnClientHelloParseEnd(void* arg);
132-
133132
static void Wrap(const v8::FunctionCallbackInfo<v8::Value>& args);
134133
static void Receive(const v8::FunctionCallbackInfo<v8::Value>& args);
135134
static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -168,9 +167,6 @@ class TLSCallbacks : public crypto::SSLWrap<TLSCallbacks>,
168167
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
169168
v8::Persistent<v8::Value> sni_context_;
170169
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
171-
172-
static size_t error_off_;
173-
static char error_buf_[1024];
174170
};
175171

176172
} // namespace node

0 commit comments

Comments
 (0)