Module refactoring and first tests

[santigimeno]

I've decided to refactor the module because doing something like this:

// pseudo-code
var ewmh = new EWMH(X, root);
ewmh.on('some_atom_event', ...);
X.SendEvent() // to request change some_atom

could lead to losing the event in the case the SendEvent was sent before the ChangeWindowAttributes on root

[sidorares]

If 'on' internally sets event mask - how SendEvent event can be lost?

[santigimeno]

Looking at the example I posted above this happens if the X server receives the SendEvent request before the ChangeWindowAttributes... so the event is never sent to the client

[sidorares]

But afaik 'on' internally calls ChangeWindowAttributes? So event mask is always updated before SendEvent?

[santigimeno]

Where exactly is it doing that? I'm xtracing my test app and I only see one ChangeWindowAttributes request: the one generated in lib/ewmh

[santigimeno]

With this sample program. It almost always sends the SendEvent request before the ChangeAttributes:

var x11 = require('x11');
var assert = require('assert');
var EWMH = require('../lib/ewmh');

x11.createClient(function(err, display) {
    if (err) {
        throw err;
    }

    var X = display.client;
    var root = display.screen[0].root

    X.InternAtom(false, '_NET_ACTIVE_WINDOW', function(err, atom) {
        if (err) {
            throw err;
        }

        var ewmh = new EWMH(X, root);
        var wid = X.AllocID();
        ewmh.once('ActiveWindow', function(w) {
            assert.equal(w, wid);
            process.exit(0);
        });

        var raw = new Buffer(32);
        raw.writeInt8(33, 0); /* ClientMessage code */
        raw.writeInt8(32, 1); /* Format */
        raw.writeUInt16LE(0, 2); /* Seq n */
        raw.writeUInt32LE(wid, 4); /* Window ID */
        raw.writeUInt32LE(atom, 8); /* Message Type */
        raw.writeUInt32LE(1, 12); /* data[0] Message from an application */
        raw.writeUInt32LE(0, 16); /* data[1]: current time */
        raw.writeUInt32LE(0, 20); /* data[2] */
        raw.writeUInt32LE(0, 24); /* data[3] */
        raw.writeUInt32LE(0, 28); /* data[4] */
        X.SendEvent(root, false, X.eventMask.SubstructureRedirect, raw);
    });
});

The relevant xtrace output:

000:<:0003: 28: Request(16): InternAtom only-if-exists=false(0x00) name='_NET_ACTIVE_WINDOW'
000:>:0003:32: Reply to InternAtom: atom=0xec("_NET_ACTIVE_WINDOW")
000:<:0004:  8: Request(3): GetWindowAttributes window=0x00000111
000:>:0004:44: Reply to GetWindowAttributes: backing-store=NotUseful(0x00) visual=0x00000021 class=InputOutput(0x0001) bit-gravity=Forget(0x00) win-gravity=NorthWest(0x01) backing-planes=0xffffffff backing-pixel=0x00000000 save-under=false(0x00) map-is-installed=true(0x01) map-state=Viewable(0x02) override-redirect=false(0x00) colormap=0x00000020 all-event-masks=0 your-event-mask=0 do-not-propagate-mask=0 unused=0x0000
000:<:0005: 44: Request(25): SendEvent propagate=false(0x00) destination=0x00000111 event-mask=SubstructureRedirect ClientMessage(33) format=0x20 window=0x00200001 type=0xec("_NET_ACTIVE_WINDOW") data=0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00;
000:<:0006: 16: Request(2): ChangeWindowAttributes window=0x00000111 value-list={event-mask=SubstructureRedirect}

[sidorares]

you are right, because ChangeWindowAttributes is in the callback of GetWindowAttributes we can send more commands before ChangeWindowAttributes is sheduled

[sidorares]

sorry, I'm a bit slow today. I understand the problem, but don't get how do you solve race condition. Can we just use GrabServer/UngrabServer ?

[santigimeno]

The idea was that If the ewmh instance is returned after we know we have sent ChangeWindowAttributes, we should be fine.

I don't know about GrabServer. I will read the spec and find out.

Thanks for the review

[sidorares]

GrabServer is the easiest way to do atomic changes - no other requests system wide allowed until you UngrabServer

[santigimeno]

Thanks. Will look into it

[santigimeno]

See https://github.com/santigimeno/node-ewmh/pull/12

Notice that GrabServer will only block requests from other connections, so the it won't solve the problem outlined in my sample program as it uses the same connection. I feel it doesn't matter as only other clients are going to SendEvent's