Thursday, June 27, 2013

Manually Cleaning Up Dart Stream Subscriptions

‹prev | My Chain | next›

I had another wildly successful #pairwithme session last night with Santiago Arias. “Wildly successful” in my mind is completing a feature in one session that I had expected to require multiple #pairwithme sessions. Alas, this means that I have to queue up the next meaty project for #pairwithme, but that is a good problem to have™.

The feature was the ability to filter projects in the ICE Code Editor:



As usual, we made extensive use of the unit test libraries in Dart to drive this feature and it all came off swimmingly. Except…

We now have approximately 496 of the following exceptions being noted in the test suite:
Exception: Bad state: No elements                dart:collection/list.dart:20
Object&ListMixin.first                           dart:collection/list.dart:20
_isEscapeKey                                     package:ice_code_editor/full.dart:259
Full._attachKeyboardHandlers.<anonymous closure> package:ice_code_editor/full.dart:165
OK, “approximately” is not the right word. “Exactly” is the right word. We have 496 of those exceptions being logged. All 126 of the tests all still pass, but they now produce those errors.

Actually, it is not 126 test that produce those errors—it is only 4 of the new tests that produce the errors. I know what is causing the errors. The stacktrace makes it pretty plain that the errors are being caused by my solution to generating keyboard events in Dart. The hacky solution that I use is to check the key identifier when the keyCode property is not available:
_isEscapeKey(e) =>
  e.keyCode == 27 || e.$dom_keyIdentifier.codeUnits.first == 27;
The reason that this popped up last night was that we used a different, TextEvent-based keyup strategy for generating input. So the obvious fix is to accommodate text events in my silly hack (or to switch from text events in our new tests).

I can address that later. What concerns me now are those errors. They are the result of keyboard listeners from old instances of the ICE editor sticking around after tests. I have been cleaning up after tests by removing the DOM element and allowing garbage collection to do the rest:
    tearDown(() {
      document.query('#ice').remove();
      editor.store..clear()..freeze();
    });
Unfortunately, it seems garbage collection is not enough in the case of Dart stream subscriptions. Instead, I have squirrel away a copy of the stream subscription so that a new, custom-built remove() method can manually remove the stream subscription:
class Full {
  // ...
  void remove() {
    _keyUpSubscription.cancel();
    el.remove();
  }
  // ...
}
With that, I can remove every instance of the DOM element removal with a call to the editor's remove() method that both removes the DOM element and the stream subscription:
    tearDown(() {
      editor.remove();
      editor.store..clear()..freeze();
    });
And that seems to do the trick.

I am not thrilled to have to do this, especially since it only seems to be of use in tests. It would be nice if stream subscriptions would be removed once the objects that created them go out of scope. Then again, the obvious case of a subscription factory would be foiled by such a strategy, so I suppose I can understand why it works this way. It makes for small pain in tests, but I can live with it.


Day #795

No comments:

Post a Comment