Windows XP crashes when ADB starts up

[bkase]

Windows XP crashes when ADB starts up. The following is an explanation and suggested solution to the problem.

The Problem

Apparently, Windows XP doesn't like it when you use thread local storage. Unfortunately, ADB uses thread local storage to ensure that JavaScript callbacks are called on the same thread that they were declared on (a requirement for js-ctypes).

The Current State of the C->JS Code

Right now, most events are triggered from C to JS with the MSG macro. The MSG macro relies on a thread-local function pointer, js_msg installed at startup for each web worker (look in the init channel of any adb-*-thread.js). MSG is defined in js_message.h, and right now only casts the data argument for you before calling the function pointer installed in each thread. MSG returns data packed in a void * which should be casted to whatever type of result you expect for that message.

On the JavaScript side, the callback installed into the function pointer is jsMsgFn which is declared just above the init listener in each of the workers. jsMsgFn is defined to be the result of the higher-order function CommonMessageHandler that first handles messages common to many listeners, and then handles the thread-specific messages that could occur (the 3rd argument). The 3rd argument to CommonMessageHandler is a (String, CData[void *]) -> CData. The first parameter is the channel for the event, the second parameter is pointer to the structure that was passed from C, and it returns a packed structure back to native code.

To extract the data from the packed structure use JsMessage.unpack : (CData[void *], CType...) -> Array[CData]. To pack a value for return back to native code use JsMessage.pack : (Number|String, (Number|String) -> (Number|String)) -> CData. The second argument for JsMessage.pack is specifically one of either global values Number or String. For example, JsMessage.pack(1, Number) or JsMessage.pack("hello", String).

There is also a dirty hack for bridging DLL calls through JavaScript used in the device IO threads and the device polling thread. I don't think this will cause any issues though, since these don't use the THREAD_LOCAL modifier.

The Fix

I think the following solution is the best in a world without thread local storage.

The MSG macro should first send data to one specific "communication" thread and then callback to JavaScript from only one thread. When the JavaScript returns a value back, we send the data back across and return it back to the thread that called MSG.

How to implement this?

The best way would probably be to reuse the fdevent infrastructure already in place in ADB.
On start-up, rather than passing the jsMsgFn callback and installing it in the native code for each thread. We can do something like the following once in the beginning of the program:

// create a pipe
int[2] sv;
adb_socketpair(sv);

// global JsMessage fd
js_message_fd = sv[0];

// create an fdevent
fdevent fde;

// install it
fdevent_install(&fde,
                sv[1],
                jsmsg_handler,
                NULL);
// callback when there is data ready to be read
fdevent_set(&fde, FDE_READ);

void jsmsg_handler(int fd, unsigned ev, void * unused) {
  if(ev & FDE_READ) {
    char channel[128];
    // grab the channel (and possibly include info about which thread?)
    if(readx(fd, channel, sizeof(channel))) {
      // error handle
    }

    void * data;
    // grab a pointer to the data
    if (readx(fd, &data, sizeof(void *))) {
      // error handle
    }

    // send the message
    void * result = js_msg(channel, data);
    // write the result back to the caller
    if (writex(fd, &result, sizeof(void *))) {
      // error handle    
    }
  }
}

Then MSG is something like:

void * js_msg'(char * channel, void * instance_ptr) {
  void * res;

  if (writex(js_message_fd, channel, strlen(channel)+1)) {
    // error handle
  }
  if (writex(js_message_fd, &instance_ptr, sizeof(void *))) {
    // error handle
  }
  if (readx(js_message_fd, &res, sizeof(void *)) {
    // error handle
  }

  return res;
}

#define MSG(channel, instance_ptr) js_msg'(channel, (void *)instance_ptr);

That way MSG acts in exactly the same way (i.e. blocking until it gets a response from JS) and we can easily communicate between threads re-using the event loop architecture that ADB already provides. This is assuming that the overhead of communicating over a socket pipe created with adb_socketpair is not high. Also, you can use blocking I/O on file descriptors that are shared cross-thread without having to have explicit mutexes.

The JavaScript side would also have to be refactored so that information still gets to the right places. Since now all the messages will come in on one web worker. See EventedChromeWorker for sending messages (optionally with responses) between ChromeWorkers.

Caveats

The fdevent_loop runs on the adb-server-thread. So if any part of adb-server-thread needs to use MSG (which it does), we will have to start a second fdevent_loop on a separate worker (and this event loop should be specifically for MSG handling for speed and thread-safety for the rest of the code). Keep in mind, that threads need to return from native code only millis later (and on Windows completely before) the web worker is terminated.

A problem that might come up is that we won't be able to pass information from MSGs into web workers that are currently in blocking native code (like the ones that called MSG for example) due to web workers' inability to block on recieving messages. The only way to get that information would be to completely return from native code back to JavaScript. In some places, this is going to be a deal-breaker and we'll have to revert back to passing function pointers around manually.

Acknowledgements

Thanks @nickdesaulniers for suggesting using one thread as the communication medium.