[KWin Scripting] Caching client objects and preventing segfaults

By ‘implementing a client system’, I mean setting up a procedural system which provides an abstract interface for converting KWin::Client* objects to QScriptValue (i.e. Wrapping) and to convert QScriptValue to KWin::Client* values (if possible) (i.e. Extracting). While deciding for an implementation of the Client system, there were two important considerations:

1. Provide a safe interface: Scripting is eventually adding an interface, an alternative way for programs (or scripts, if you will) to call functions and read variable members of C++ objects. Despite how we look at the whole implementation, at the lowest level of abstraction, this is how it is. However, scripting is not, if I must put it in words, a very serious enterprise. While writing and executing scripts, we surely expect objects where it’s properties are undefined and they don’t do anything, but those are the cases where a script fails. However, we definitely don’t want an application to crash because of a script gone bad. And when the application is your window manager, that is one thing we sincerely don’t want.
2. Reduce wrapping: C++ objects, specifically, KWin’s C++ objects are wrapped only when such a need arises. For example, if a specific event is triggered which passes a Client object to it’s registered functions, then while calling such a function, those script objects are wrapped (within the code, I refer to it as ‘generating’). In certain cases, multiple events have to provide the same object i.e. an object which eventually wraps the same client in memory. Take the case of a method, workspace.getAllClients(). This event returns an array of ‘clients’ in the order on their stacking. Have a look at this piece of code:

     var list_one = workspace.getAllClients();
     var list_two = workspace.getAllClients();

   In this case, the list elements are exactly the same, and there will be two sets of script objects in memory for each client, however, despite the fact that the wrapping is same for both objects, without caching the same clients will be generated twice. With a lot of events registered, during it’s lifetime, a single client will go through a lot of events:

   • clientAdded
   • clientRestored / clientMinimized
   • client.moved / client.geometryChanged

   Without caching, these objects will be generated every time for each event that requires them, hence adding to a lot of computational overhead. As more and more events and methods added to these client objects, generating a client becomes a more and more computationally expensive process.

Talking about the event client.moved() or client.geometryChanged(), during a client drag process, these functions are being called very frequently, so much as around 5 – 10 second^-1. To overcome this problem and even to take care of segmentation faults, the method used is outlined as below:

• A list is maintained which maps every KWin::Client* object to a SWrapper::Client* object and a QScriptValue. (A KWin::Client* is the client object within KWin and an SWrapper::Client* is the QObject which acts as the wrapper for the object). A SWrapper::Client* object points to a KWin::Client* object that it wraps, but not to a QScriptValue as it isn’t even necessary. The implementation is such that a KWin::Client* maps to both a SWrapper::Client* object and a QScriptValue. A simple diagramatic view is given below:

• Whenever a new object is to be generated for a given QScriptEngine, say ‘eng’, the method eng.toScriptValue(client) must be called where client is of type KWin::Client*. KWin::Client* is registered as a metatype with Qt and also it’s scriptvalue conversion functions, so for the main engine used, it understands objects of type KWin::Client*. Do note here that KWin::Client is not wrapped, a pointer to it i.e. KWin::Client* is. The metatype registered is NOT KWin::Client, but KWin::Client*. During conversion, first a call is made to resolveClient. The resolveClient(KWin::Client* client) methods searches the clientMap for the existence of a mapped SWrapper::Client* and a QScriptValue. The return value is of type ClientResolution, which is nothing but a QPair<SWrapper::Client*, QScriptValue>. If a mapped object is not found, then a default ClientResolution is returned with a null pointer and an invalid QScriptValue i.e. ClientResolution(0, QScriptValue()). If a non-default ClientResolution is returned, it means such a value has already been generated and hence, the stored QScriptValue is returned. If a default ClientResolution is returned on the other hand, then the given KWin::Client* is wrapped resulting in an SWrapper::Client* and a QScriptValue. Both these values are then added to the clientMap to take care of future caching requests.
• In my first implementation, I used a structure { KWin::Client* client; SWrapper::Client* wrapper; QScriptValue value; }, however, every resolution used linear search and hence lookups were not very ideal. To overcome that, the current clientMap is a QHash<KWin::Client*, QPair<SWrapper::Client*, QScriptValue> >. The lookups are faster and the storage is much more convinient.

Take the case of the following QtScript code:

var captionList = workspace.getAllClients();
var captionList2 = workspace.getAllClients();

print(typeof captionList);

for(var i in captionList) {
    print(i + ":" + captionList[i].caption());
}

for(var i in captionList) {
    print(i + ":" + captionList2[i].caption());
}

function added(client) {
    print("Client added [" + client.caption() + "]");
}

function removed(client) {
    print("Client is being removed [" + client.caption() + "]");
}

workspace.clientAdded.connect(added);
workspace.clientRemoved.connect(removed);

I also modified the client wrapper code a bit, such that on using QScriptEngine::toScriptValue(KWin::Client*), it prints whether a cached value is being returned or a new one is being generated, alongwith the client’s caption. The following was the output.

Generating a new one [ "plasma-desktop" ]
Generating a new one [ "kde-devel@37MeanMachine:~" ]
Generating a new one [ "kwin : kwin" ]
Generating a new one [ "plasma-desktop" ]
Using cached [ "plasma-desktop" ]
Using cached [ "kde-devel@37MeanMachine:~" ]
Using cached [ "kwin : kwin" ]
Using cached [ "plasma-desktop" ]
0:plasma-desktop
....
0:plasma-desktop
....
3:plasma-desktop
** I Opened the application launcher here
Generating a new one [ "plasma-desktop" ]
Client added [plasma-desktop]
Using cached [ "plasma-desktop" ]
Client is being removed [plasma-desktop]
Generating a new one [ "Kate" ]
Client added [Kate]
Using cached [ "Kate" ]
Generating a new one [ "Save File – Kate" ]
Client added [Save File – Kate]
Using cached [ "Save File – Kate" ]
Client is being removed [Save File – Kate]

If you look, for the first workspace.getAllClients(), each client was generated. However for the second run, cached values were generated. And both of them work just the way they were supposed to. Next, I opened and closed the plasma application launcher. First, a new QScriptValue was generated for the launcher window, but for the removing event, the cached value was used. After that, I just opened a new Kate window and played around with the Save File dialog. Also, note here that for both the Client objects, pointers are used, such is not the case with QScriptValue. For each wrapping, if a cached value is found, then the value returned is a copy and not a reference. As such, in the above case, captionList[3] and captionList2[3] are two different objects, but they refer to the same KWin::Client object. If I make changes to captionList[3] via scripting (for ex. add a property), those changes will not be refelected in captionList2[3], as is the expected behaviour. Client objects are floating objects i.e. objects of this type cannot be created within a script. They have to be received as parameters of functions or return values of methods. Hence, these objects are given ScriptOwnership, so that the garbage collector of the script engine takes care of these variables.

Preventing segmentation faults: To prevent segmentation faults, we first need to see how a QScriptValue maps to a KWin::Client*. To do so, the pointer to KWin::Client is stored within the QScriptValue using the QScriptValue::data() function. On retrieval, this very pointer is retrieved. Let’s take an example:

1. I register a function register with the event workspace.clientAdded and a function alert with workspace.activeDesktopChanged :

   var x;
   
   function register(client) {
       x = client;
   }
   
   function alert(desk_no) {
       print(x.caption());
   }
   
   workspace.clientAdded.connect(register);
   workspace.activeDesktopChanged.connect(alert);

2. What happens is that the value of ‘x’ stores the client added most recently and then whenever a desktop is changed that stored client’s caption is printed. Now let’s say that I start a window and close it. Here, ‘x’ still holds the value of the pointer to the object that has been deleted. Thereby any call to the function ‘alert’ will result in a segmentation fault!

To prevent the same, the destructor of KWin::Client, calls the SWrapper::Client::releaseClient(KWin::Client*) function with parameter ‘this’, which results in the entry with that particular client’s pointer as key to be removed from the clientMap. Also, every C++ function which is mapped to a javascript function performs a check during extracting the KWin::Client pointer to make sure whether it exists in the clientMap or not. If it doesn’t, the function returns an invalid value. Have a look at the SWrapper::Client::caption() function:

KWin::Client* SWrapper::Client::extractClient(const QScriptValue& value) {
    KWin::Client* client = static_cast<KWin::Client*>(qscriptvalue_cast<void*>(value.data()));

    if((clientResolve(client)).first != 0) {
	return client;
    } else {
	return 0;
    }
}

QScriptValue SWrapper::Client::caption(QScriptContext* ctx, QScriptEngine* eng) {
   KWin::Client* central = extractClient(ctx->thisObject());

    if(central == 0) {
	return QScriptValue();
    } else {
	return eng->toScriptValue(central->caption());
    }
}

In our previous example, after the client is deleted, so is it’s entry from the clientMap, in which case the alert function calls a member function with it’s ‘this’ object containing an invalid pointer, which is detected by the extractClient method and hence by returning a null pointer, which is not acted upon, prevents a segmentation fault.

For more of this code, refer to ‘scripting/client.cpp’ and ‘scripting/meta.cpp’ in WebSVN for the kwin_scripting branch.