libuv uv_async_send help?

[J Decker]

I'm working on a simple network proxy addon for node (yes it's sort of a crossover issue, but the error happens in ... hmm no adding run fixed that... moved the problem I guess

I cut out most of the excess code; there's still excess in the custructor callback... but I wanted to leave that flow intact.

when creating a new object, I create a single uv_default_loop() that I'll never close ?

on destructor I do  uv_close( (uv_handle_t*)&async, NULL );

uv_run( fbdl.loop, UV_RUN_DEFAULT );

 

adding the run stops it from generating an assert(0) about a bad handle block in the loop (I guess, i forget the header)

Assertion failed: 0, file c:\ws\deps\uv\src\win\handle-inl.h, line 159

probably this version is close https://github.com/mapbox/node/blob/master/deps/uv/src/win/handle-inl.h

from a standard thread created with CreateThread in windows I call 

uv_async_send( &state->object->async );

I don't call  uv_async_send( &state->object->async );

 after the object destructs or anywhere near it... the sockets have long been closed and the object can be garbage collected because no more events will happen to it....

After running the 'uv_run' mentioend above, now node.exe is jumping to address 00000000 like somehow it lost a callback.  It's a fatal crash and node just exits... 

----------------------

void AppComObject::Init( Handle<Object> exports ) {

    // init's stuff

}

//--------------- Here is the callback ------- 

// something i'm not cleaning up here??

void asyncmsg( uv_async_t* handle ) {

// Called by UV in main thread after our worker thread calls uv_async_send()

//    I.e. it's safe to callback to the CB we defined in node!

v8::Isolate* isolate = v8::Isolate::GetCurrent();

AppComObject* myself = (AppComObject*)handle->data;

HandleScope scope(isolate);

{

struct _MSGBUF *msg;

while( msg = (struct _MSGBUF *)DequeLink( &myself->netstate->network_receive_queue ) ) {

Local<Value> object = ProcessCommand( isolate, myself->netstate, msg, TRUE );

Local<Value> argv[] = { object };

Local<Function> cb = Local<Function>::New( isolate, myself->cbEvent );

cb->Call( myself->jsThis, 1, argv );

}

}

}

AppComObject::AppComObject() {

lprintf( "Create new com object %p", this );

}

void AppComObject::New( const FunctionCallbackInfo<Value>& args ) {

Isolate* isolate = args.GetIsolate();

if( args.IsConstructCall() ) {

Handle<Function> arg0 = Handle<Function>::Cast( args[0] );

Persistent<Function> cb( isolate, arg0 );

// Invoked as constructor: `new MyObject(...)`

AppComObject* obj = new AppComObject(  );

obj->cbEvent = cb;

if( !fbdl.loop ) // create one uv_default_loop that I keep forever???

fbdl.loop = uv_default_loop();

// had to init the async struct to 0 otherwise init din't work right?

MemSet( &obj->async, 0, sizeof( obj->async ) );

uv_async_init( fbdl.loop, &obj->async, asyncmsg );

obj->async.data = obj;

lprintf( "new async..." );

obj->Wrap( obj->jsThis = args.This() );

args.GetReturnValue().Set( args.This() );

}

else {

// Invoked as plain function `MyObject(...)`, turn into construct call.

int argc = args.Length();

Local<Value> *argv = new Local<Value>[argc];

for( int n = 0; n < argc; n++ )

argv[n] = args[n];

Local<Function> cons = Local<Function>::New( isolate, constructor );

args.GetReturnValue().Set( cons->NewInstance( argc, argv ) );

delete argv;

}

}

AppComObject::~AppComObject() {

lprintf( "object evaporated. %p", this );

uv_close( (uv_handle_t*)&async, NULL );

uv_run( fbdl.loop, UV_RUN_DEFAULT );

}

NODE_MODULE( app_com_module, AppComObject::Init )

[Saúl Ibarra Corretgé]

On 09/01/2016 01:06 AM, J Decker wrote:
> I'm working on a simple network proxy addon for node (yes it's sort of a
> crossover issue, but the error happens in ... hmm no adding run fixed
> that... moved the problem I guess
>
> I cut out most of the excess code; there's still excess in the
> custructor callback... but I wanted to leave that flow intact.
>

Was there a previous email I missed? I have no context thus far.

> when creating a new object, I create a single uv_default_loop() that
> I'll never close ?
>

That's going to be a problem. Node itself uses the default loop, so
you'd be using the same loop really, and uv_run is non-recursive, so you
cannot call it while it's already running, and it is, otherwise your JS
code wouldn't run.

> on destructor I do uv_close( (uv_handle_t*)&async, NULL );
> uv_run( fbdl.loop, UV_RUN_DEFAULT );
>

As mentioned above, bad idea. Even if you use a new loop, why? Why not
use the existing loop? Are you also using other threads?

> adding the run stops it from generating an assert(0) about a bad handle
> block in the loop (I guess, i forget the header)
> Assertion failed: 0, file c:\ws\deps\uv\src\win\handle-inl.h, line 159
> probably this version is
> close https://github.com/mapbox/node/blob/master/deps/uv/src/win/handle-inl.h
>
> from a standard thread created with CreateThread in windows I call
> uv_async_send( &state->object->async );
>

That is OK, uv_async_send is thread-safe.

> I don't call uv_async_send( &state->object->async );
> after the object destructs or anywhere near it... the sockets have long
> been closed and the object can be garbage collected because no more
> events will happen to it....
>
> After running the 'uv_run' mentioend above, now node.exe is jumping to
> address 00000000 like somehow it lost a callback. It's a fatal crash
> and node just exits...
>

[snip]

I haven't looked at the code, but you should just create the handle in
the loop which is attached to the current Environment, then just close
the handle when you're done with it. Note that it's not OK to free the
memory for the handle right after the call to uv_close, you need to pass
a callback and free it there.


Cheers,

--
Saúl Ibarra Corretgé
bettercallsaghul.com

[J Decker]

I didn't get an email notice and I've checked and it says all over 'get email notifications' 

[J Decker]

On Thursday, September 1, 2016 at 12:54:20 AM UTC-7, Saúl Ibarra Corretgé wrote:

On 09/01/2016 01:06 AM, J Decker wrote:
> I'm working on a simple network proxy addon for node (yes it's sort of a
> crossover issue, but the error happens in ... hmm no adding run fixed
> that... moved the problem I guess
>
> I cut out most of the excess code; there's still excess in the
> custructor callback... but I wanted to leave that flow intact.
>

Was there a previous email I missed? I have no context thus far.

No; that's about all the context I can give? I can provide a more complete example, and probably should.

Unfortunatly I'm under windows at the moment, so I don't even know what the callback stack looks like; working on building it under linux today...

 

> when creating a new object, I create a single uv_default_loop() that
> I'll never close ?
>

That's going to be a problem.  Node itself uses the default loop, so
you'd be using the same loop really, and uv_run is non-recursive, so you
cannot call it while it's already running, and it is, otherwise your JS
code wouldn't run.

uv_run is only called during garbage collection, which should be the same thread that is the javascript?

why would one want to use multiple loops?  and add a loop per async then?

 

> on destructor I do uv_close( (uv_handle_t*)&async, NULL );
> uv_run( fbdl.loop, UV_RUN_DEFAULT );
>  

As mentioned above, bad idea.  Even if you use a new loop, why?  Why not
use the existing loop?  Are you also using other threads?

So I shouldn't use multple Async objects each with their own .data referencing a object?

is the exsting loop ' get_default_loop()' so I shouldn't close it?  I never do?

 

[snip]

I haven't looked at the code, but you should just create the handle in
the loop which is attached to the current Environment, then just close
the handle when you're done with it.  Note that it's not OK to free the
memory for the handle right after the call to uv_close, you need to pass
a callback and free it there.

which is why I did it in the destructor; immediately after returning the memory is freed.

[Saúl Ibarra Corretgé]

On 02/09/16 19:31, J Decker wrote:
>
> On Thursday, September 1, 2016 at 12:54:20 AM UTC-7, Saúl Ibarra
> Corretgé wrote:
>
> On 09/01/2016 01:06 AM, J Decker wrote:
> > I'm working on a simple network proxy addon for node (yes it's
> sort of a
> > crossover issue, but the error happens in ... hmm no adding run fixed
> > that... moved the problem I guess
> >
> > I cut out most of the excess code; there's still excess in the
> > custructor callback... but I wanted to leave that flow intact.
> >
>
> Was there a previous email I missed? I have no context thus far.
>
> No; that's about all the context I can give? I can provide a more
> complete example, and probably should.
> Unfortunatly I'm under windows at the moment, so I don't even know what
> the callback stack looks like; working on building it under linux today...
>

Your email sounded like a conversation with someone, and I thought I was
just getting one side of it :-P

>
> > when creating a new object, I create a single uv_default_loop() that
> > I'll never close ?
> >
>
> That's going to be a problem. Node itself uses the default loop, so
> you'd be using the same loop really, and uv_run is non-recursive, so
> you
> cannot call it while it's already running, and it is, otherwise your JS
> code wouldn't run.
>
>
> uv_run is only called during garbage collection, which should be the
> same thread that is the javascript?

Yes, but that's incorrect, you should not call uv_run yourself.

> why would one want to use multiple loops? and add a loop per async then?
>

It depends on the use case. In yours, you probably want multiple async
handles.

>
> > on destructor I do uv_close( (uv_handle_t*)&async, NULL );
> > uv_run( fbdl.loop, UV_RUN_DEFAULT );
> >
>
> As mentioned above, bad idea. Even if you use a new loop, why? Why
> not
> use the existing loop? Are you also using other threads?
>
> So I shouldn't use multple Async objects each with their own .data
> referencing a object?

Yes.

> is the exsting loop ' get_default_loop()' so I shouldn't close it? I
> never do?
>

Yes, you can get it from the current Environment object, then call
event_loop() on it. Example:
https://github.com/nodejs/node/blob/1b99093df78b795052c944fc6388a934e84e89ef/src/timer_wrap.cc#L99-L100

Never close the loop, Node does it on its own.


--
Saúl Ibarra Corretgé
http://bettercallsaghul.com

[J Decker]

the previous message on this group specifes you must after calling close. (I see but if I never close the async, and of course I can't use data for anything useful and it might as well be null and just reference a static container that contains a queue with the object who's callback is to be called for the message received?

 

why would one want to use multiple loops?  and add a loop per async then?

It depends on the use case. In yours, you probably want multiple async handles.

I really think I do.... but if I don't call run after close it libuv genrates an assert(0) ; yes I'm running a debug build and if I built it release it wouldn't crash?  But if I continue and ignore the assert it does crash anyway.

 

    > on destructor I do uv_close( (uv_handle_t*)&async, NULL );
    > uv_run( fbdl.loop, UV_RUN_DEFAULT );
    >

    As mentioned above, bad idea.  Even if you use a new loop, why?  Why
    not
    use the existing loop?  Are you also using other threads?

So I shouldn't use multple Async objects each with their own .data
referencing a object?

Yes.

You just said 'you probably want multiple async handles' and to 'so I should NOT use multiple async objects" you say yes?

I phrased it differently so I could get a clear answer.

 

is the exsting loop ' get_default_loop()' so I shouldn't close it?  I
never do?

Yes, you can get it from the current Environment object, then call event_loop() on it.  Example: https://github.com/nodejs/node/blob/1b99093df78b795052c944fc6388a934e84e89ef/src/timer_wrap.cc#L99-L100

Never close the loop, Node does it on its own.

"I never do. " sorry the question mark was ... sarcasm/rhetoric.

but ok I can use the one from the environment.

--
Saúl Ibarra Corretgé
http://bettercallsaghul.com

--
You received this message because you are subscribed to a topic in the Google Groups "libuv" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/libuv/XXOovowH8EE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to libuv+unsubscribe@googlegroups.com.
To post to this group, send email to li...@googlegroups.com.
Visit this group at https://groups.google.com/group/libuv.
For more options, visit https://groups.google.com/d/optout.

[J Decker]

On Fri, Sep 2, 2016 at 2:21 PM, J Decker <d3c...@gmail.com> wrote:

On Fri, Sep 2, 2016 at 1:53 PM, Saúl Ibarra Corretgé <sag...@gmail.com> wrote:

why would one want to use multiple loops?  and add a loop per async then?

It depends on the use case. In yours, you probably want multiple async handles.

I really think I do.... but if I don't call run after close it libuv genrates an assert(0) ; yes I'm running a debug build and if I built it release it wouldn't crash?  But if I continue and ignore the assert it does crash anyway.

 

    > on destructor I do uv_close( (uv_handle_t*)&async, NULL );
    > uv_run( fbdl.loop, UV_RUN_DEFAULT );
    >

    As mentioned above, bad idea.  Even if you use a new loop, why?  Why
    not
    use the existing loop?  Are you also using other threads?

So I shouldn't use multple Async objects each with their own .data
referencing a object?

Yes.

You just said 'you probably want multiple async handles' and to 'so I should NOT use multiple async objects" you say yes?

I phrased it differently so I could get a clear answer.

 

is the exsting loop ' get_default_loop()' so I shouldn't close it?  I
never do?

Yes, you can get it from the current Environment object, then call event_loop() on it.  Example: https://github.com/nodejs/node/blob/1b99093df78b795052c944fc6388a934e84e89ef/src/timer_wrap.cc#L99-L100

Under windows, this doesn't link,  .... env.h is not a header that's provided for addon modules.

and if I steal the .h, there's no library to provide 'Environemt::event_loop()

[J Decker]

(truncated)

if I create a single loop I use forever, and do not call uv_run() after calling uv_close()   after a few connections (object creation/deletions)  This is the stack trace under linux.

Under windows I get the assert I mentioned at the start after the second connection.

-------------------

node: src/unix/core.c:236: uv__finish_close: Assertion `handle->flags & UV_CLOSING' failed.

Thread 1 "node" received signal SIGABRT, Aborted.

#0  0x00007ffff565704f in raise () from /usr/lib/libc.so.6

#1  0x00007ffff565847a in abort () from /usr/lib/libc.so.6

#2  0x00007ffff564fea7 in __assert_fail_base () from /usr/lib/libc.so.6

#3  0x00007ffff564ff52 in __assert_fail () from /usr/lib/libc.so.6

#4  0x00007ffff77a2f10 in uv_run () from /usr/lib/libuv.so.1

#5  0x0000000000d71a70 in node::Start(int, char**) ()

#6  0x00007ffff5644291 in __libc_start_main () from /usr/lib/libc.so.6

#7  0x00000000006c7d3a in _start ()

(gdb)

---------------------

If I add the uv_run after uv_close after a few more connections this is the stack trace...

 at frame 15 #15 0x00007ffff1780e07 in asyncmsg (handle=0x153e8f0) at 

it's gotten a message from the network, and is building a V8 object, and triggers garbage collection which falls off into nowhere.

And I don't know; maybe there's a irregularity in the stack and it's not resolving correctly?

#0  0x0000000000000000 in ?? ()

#1  0x000000000099d62e in v8::internal::GlobalHandles::PendingPhantomCallback::Invoke(v8::internal::Isolate*) ()

#2  0x000000000099d7ca in v8::internal::GlobalHandles::DispatchPendingPhantomCallbacks(bool) ()

#3  0x000000000099d95a in v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector, v8::GCCallbackFlags) ()

#4  0x00000000009b85f2 in v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) ()

#5  0x00000000009b8f73 in v8::internal::Heap::CollectGarbage(v8::internal::GarbageCollector, char const*, char const*, v8::GCCallbackFlags) ()

#6  0x000000000097720b in v8::internal::Factory::NewUninitializedFixedArray(int) ()

#7  0x000000000096cbce in ?? ()

#8  0x000000000096d34f in ?? ()

#9  0x0000000000a724c2 in v8::internal::JSObject::AddDataElement(v8::internal::Handle<v8::internal::JSObject>, unsigned int, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes, v8::internal::Object::ShouldThrow) ()

#10 0x0000000000a9b0a8 in v8::internal::Object::AddDataProperty(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes, v8::internal::Object::ShouldThrow, v8::internal::Object::StoreFromKeyed) ()

#11 0x0000000000ab3bff in v8::internal::Object::SetProperty(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::LanguageMode, v8::internal::Object::StoreFromKeyed) ()

#12 0x00000000006eafba in v8::Object::Set(v8::Local<v8::Context>, unsigned int, v8::Local<v8::Value>) ()

#13 0x00000000006eb0cd in v8::Object::Set(unsigned int, v8::Local<v8::Value>) ()

#14 0x00007ffff1783aaa in ProcessCommand (isolate=0x14ead70, state=0x7fffec1582d4, msg=0x7fffec2b6a24, bTCP=1)

    at /home/node/app_service/AppCom/flashboard_net.cc:265

#15 0x00007ffff1780e07 in asyncmsg (handle=0x153e8f0) at /home/node/app_service/AppCom/appcom.cc:70

#16 0x00007ffff77a22a3 in ?? () from /usr/lib/libuv.so.1

#17 0x00007ffff77a2386 in ?? () from /usr/lib/libuv.so.1

#18 0x00007ffff77b11f8 in ?? () from /usr/lib/libuv.so.1

#19 0x00007ffff77a2c44 in uv_run () from /usr/lib/libuv.so.1

#20 0x00007ffff178197f in AppComObject::~AppComObject (this=0x16db410, __in_chrg=<optimized out>)

    at /home/node/app_service/AppCom/appcom.cc:127

#21 0x00007ffff17819f0 in AppComObject::~AppComObject (this=0x16db410, __in_chrg=<optimized out>)

    at /home/node/app_service/AppCom/appcom.cc:128

#22 0x00007ffff178225c in node::ObjectWrap::WeakCallback (data=...)

    at /root/.cmake-js/node-x64/v6.3.1/include/node/node_object_wrap.h:103

#23 0x000000000099d62e in v8::internal::GlobalHandles::PendingPhantomCallback::Invoke(v8::internal::Isolate*) ()

#24 0x000000000099d7ca in v8::internal::GlobalHandles::DispatchPendingPhantomCallbacks(bool) ()

#25 0x000000000099d95a in v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector, v8::GCCallbackFlags) ()

#26 0x00000000009b85f2 in v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) ()

#27 0x00000000009b8f73 in v8::internal::Heap::CollectGarbage(v8::internal::GarbageCollector, char const*, char const*, v8::GCCallbackFlags) ()

#28 0x0000000000976c3a in v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, char const*, v8::GCCallbackFlags) ()

#29 0x000000000097e24d in v8::internal::Factory::NewRawOneByteString(int, v8::internal::PretenureFlag) ()

#30 0x0000000000a82aba in v8::internal::String::SlowFlatten(v8::internal::Handle<v8::internal::ConsString>, v8::internal::PretenureFlag) ()

#31 0x00000000006cedf0 in v8::internal::String::Flatten(v8::internal::Handle<v8::internal::String>, v8::internal::PretenureFlag)

    ()

[J Decker]

do NOT call uv_close_async() during the addon's object destructor.

I moved it to the function called just before the object should evaporate.

removed the uv_run; and all is well;