Re: [firebreath-dev] Firing an event with a reference to a FB::JSObject

[Richard Bateman]

Comments in-line

On Jul 1, 2012, at 17:12 , Adrian C wrote:

> I have created a collection of scriptable objects by exposing in MyPluginAPI a property that returns an array, initialized like this:
>
> m_collection = m_host->getDOMWindow()->createArray();
>
> Then thanks to Richard, I learned how to push my custom objects into that array. I just defined my new class derived from JSAPIAuto and do this:
>
> MyObjectPtr newMyObject = boost::make_shared<MyObject>(m_host, [...other constructor parameters...]));
> m_collection->Invoke("push", FB::variant_list_of(newMyObject));
>
> All this works very well. I can browser the collection from JavaScript, use properties and invoke methods in the objects in the collection.
>
> Now I have a working thread that monitors changes and when they happen I want to fire an event that includes a reference to one of the items in the above collection. The working thread has access to a native C array provided by the OS API and the array defines an LPVOID member for each element of the array. So I wanted to use that pointer to reference each of the objects and when the event happens, try to use it in the call.
>
> First, to pass the objects from the main thread to the working thread, I created a shared_ptr to the original MyObjectPtr to cast it to LPVOID. I am not sure whether this is necessary or valid.
>
> MyObjectPtr *ppMyObject = new MyObjectPtr(newMyObject);
> m_rgNative[i].pvUserData = (void*)ppMyObject;

This is unneccesary and potentially dangerous. The reference counting on boost::shared_ptr is threadsafe, though destruction is not. That means that you're fine to access the ptr from another thread as long as you're careful with it (all normal rules for accessing the same object on multiple threads apply) but make sure that the last release happens on the main thread.

> Then in the workin thread, I try to get the pointer back to retrieve the reference to the object. I know that in COM you can't pass COM objects across threads without complex marshaling. Does one of the many fantastic features of Firebreath includes taking care of this?
>
> MyObjectPtr *ppMyObject = (MyObjectPtr*)(m_rgNative[i].pvUserData);
> MyObjectPtr theMyObject = *ppMyObject;
> delete ppMyObject;
> theMyObject->set_someproperty(value); <- this seems to work.

You're really over thinking this; it's just an object. This isn't COM, so you don't have to worry about that. However, make sure that anything that may be accessed from javascript or from your alternate thread are threadsafe.

> The last statement above seems to work, I can actually set properties in my custom object across threads. But I am not sure again if it's valid. Besides, I think it fails if the object is busy, so it might not be the correct solution to the interthread communication.

You can probably solve a lot of the potential issues simply by throwing a mutex into your getters, setters, and methods.

> And then, last, I have no clue how to reference the original object in the fire event method. For the time being, it's working fine if the event is defined as:
>
> FB_JSAPI_EVENT(statuschange, 2, (const FB::variant&, const int));
> and fired from the working thread calling

This should work fine; FB::variant is actually what it will end up being. If you do this then you can pass whatever (supported) type you want for the first parameter.

> fire_statuschange(std::wstring(m_rgNative[i].szName), m_rgNative[i].dwCurrentState);
>
> But since I don't want a string, but a reference to the scriptable object, I tried to change the event definition to:
>
> FB_JSAPI_EVENT(statuschange, 2, (const FB::JSObjectPtr&, const int));
> and fired from the working thread calling
> fire_statuschange(theMyObject, m_rgNative[i].dwCurrentState);

This should work as well; if it doesn't, truy passing just a FB::JSObjectPtr without the const &; I don't know of a reason that the const & wouldn't work, but that's the only explanation I can think of.

One of your biggest issues is that you are seriously over-thinking the cross thread stuff. Basically when you're dealing with multiple threads in normal C++ the only real concern is that you could have two things accessing it at the same time; if one changes something while another is running then you could run into some problems, and you could definitely have an issue if the last two shared_ptr instances go away at the same time on different threads because they could both try to destruct your object, but otherwise it's just a variable in memory shared between both threads. Note that fire_* will always fire that event on the main thread asynchronously.

An FB::JSObjectPtr object will also automatically cause all calls that modify data to happen on the main thread; this is usually good, but be aware that if you are doing a lot of operations one after another from a different thread you may want to wrap them all in a function and use m_host->CallOnMainThread to call it; each call that happens on the main thread adds a reasonably significant amount of delay to the call, so you can run into performance problems that way if you aren't careful.

Hope that helps,

Richard

[Adrian C]

Richard, as always, thanks for the insight. But I might be missing something from your suggestions. Maybe you can spot the error?

    MyObjectPtr newObject = boost::make_shared<MyObject>(m_host, ...);

    // I was making a pointer to a pointer like MyObjectPtr *ppObject = new MyObjectPtr(newObject);

    // but changed as per your suggestion to just the native pointer. I added get() otherwise I can't convert when compiling

    m_Native[i].pvUserData = (void*)newObject.get();

But now I fear that when newObject goes out of scope, the raw pointer is no longer useful. Should I use some other method in the boost::shared_ptr to increase the reference count?

But also, before I dereferenced the pointer to a pointer to create a new variable in my other method, I don't know how to obtain again a MyObjectPtr from the raw pointer, just using this won't compile:

    MyObjectPtr theObject = MyObjectPtr(m_Native[i].pvUserData);

So what would be the correct conversion in the second method?

Thanks a lot!

-Adrian-

On Monday, July 2, 2012 9:12:33 AM UTC+10, Adrian C wrote:

I have created a collection of scriptable objects by exposing in MyPluginAPI a property that returns an array, initialized like this:

 

m_collection = m_host->getDOMWindow()->createArray();

 

Then thanks to Richard, I learned how to push my custom objects into that array. I just defined my new class derived from JSAPIAuto and do this:

 

    MyObjectPtr newMyObject = boost::make_shared<MyObject>(m_host, [...other constructor parameters...]));
    m_collection->Invoke("push", FB::variant_list_of(newMyObject));

 

All this works very well. I can browser the collection from JavaScript, use properties and invoke methods in the objects in the collection.

 

Now I have a working thread that monitors changes and when they happen I want to fire an event that includes a reference to one of the items in the above collection. The working thread has access to a native C array provided by the OS API and the array defines an LPVOID member for each element of the array. So I wanted to use that pointer to reference each of the objects and when the event happens, try to use it in the call.

 

First, to pass the objects from the main thread to the working thread, I created a shared_ptr to the original MyObjectPtr to cast it to LPVOID. I am not sure whether this is necessary or valid.

 

    MyObjectPtr *ppMyObject = new MyObjectPtr(newMyObject);
    m_rgNative[i].pvUserData = (void*)ppMyObject;

 

Then in the workin thread, I try to get the pointer back to retrieve the reference to the object. I know that in COM you can't pass COM objects across threads without complex marshaling. Does one of the many fantastic features of Firebreath includes taking care of this?

 

    MyObjectPtr *ppMyObject = (MyObjectPtr*)(m_rgNative[i].pvUserData);
    MyObjectPtr theMyObject = *ppMyObject;
    delete ppMyObject;
    theMyObject->set_someproperty(value); <- this seems to work.

 

The last statement above seems to work, I can actually set properties in my custom object across threads. But I am not sure again if it's valid. Besides, I think it fails if the object is busy, so it might not be the correct solution to the interthread communication.

 

And then, last, I have no clue how to reference the original object in the fire event method. For the time being, it's working fine if the event is defined as:

 

    FB_JSAPI_EVENT(statuschange, 2, (const FB::variant&, const int));

and fired from the working thread calling

 

    fire_statuschange(std::wstring(m_rgNative[i].szName), m_rgNative[i].dwCurrentState);

But since I don't want a string, but a reference to the scriptable object, I tried to change the event definition to:

 

     FB_JSAPI_EVENT(statuschange, 2, (const FB::JSObjectPtr&, const int));

and fired from the working thread calling

     fire_statuschange(theMyObject, m_rgNative[i].dwCurrentState);

 

Which doesn't work, probably because I'm doing it wrong and hoping for some magic conversion. Can anyone help me understanding what is the correct way to do both things: pass the object reference across threads and then pass it back out as the parameter for the event?

 

 

[Richard Bateman]

On Jul 3, 2012, at 16:46 , Adrian C wrote:

> Richard, as always, thanks for the insight. But I might be missing something from your suggestions. Maybe you can spot the error?
>
> MyObjectPtr newObject = boost::make_shared<MyObject>(m_host, ...);
> // I was making a pointer to a pointer like MyObjectPtr *ppObject = new MyObjectPtr(newObject);
> // but changed as per your suggestion to just the native pointer. I added get() otherwise I can't convert when compiling
> m_Native[i].pvUserData = (void*)newObject.get();
>
> But now I fear that when newObject goes out of scope, the raw pointer is no longer useful. Should I use some other method in the boost::shared_ptr to increase the reference count?

You're right, the raw pointer at that point would be useless; this is why you should never ever do that =] There is no good reason to cast it to a void*. Why would you do that? Just leave it as a MyObjectPtr (boost::shared_ptr<MyObject>).

>
> But also, before I dereferenced the pointer to a pointer to create a new variable in my other method, I don't know how to obtain again a MyObjectPtr from the raw pointer, just using this won't compile:
>
> MyObjectPtr theObject = MyObjectPtr(m_Native[i].pvUserData);
>
> So what would be the correct conversion in the second method?
>
> Thanks a lot!

I'm not even going to answer this question; it's possible to do, but it would be a very bad idea, because there would be no reference to the original shared_ptr and you'd then have two shared_ptr instances trying to manage the lifecycle -- this is a guaranteed way to crash your plugin.

Just make sure that your second thread goes away before your main reference to MyObjectPtr goes away and you should be fine. There is no reason to pass things across threads as a void*.

Richard

[Adrian C.]

The reason for requiring a void* is that the operating system callback function that would provide me information when the even happens is a native C function and the only thing that it would provide me to hook back into my code is that pvUserData structure member.

What I would like to do is remove a messy workaround where I build the native array of struct for the system call and then in JavaScript I find the element in the scriptable array by index.

So I guess the question ultimately is: If a native function that defines a LPVOID member needs to reference a JSObjectPtr, what is the recomended way to perform the conversion back and forth? (or eventually use some pattern of Firebreath constructs to manage that case)