Skip to content

Commit

Permalink
Fix a race condition in meta.load-css() (#1376)
Browse files Browse the repository at this point in the history
We weren't properly awaiting a call to CssStylesheet.accept(), which
meant that it could try to continue doing work asynchronously in the
wrong context.

Closes #1318
  • Loading branch information
nex3 committed Jun 24, 2021
1 parent 713b7cc commit 83343d7
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -16,6 +16,9 @@
* Fix an edge case where `@extend` wouldn't affect a selector within a
pseudo-selector such as `:is()` that itself extended other selectors.

* Fix a race condition where `meta.load-css()` could trigger an internal error
when running in asynchronous mode.

## 1.35.1

* Fix a bug where the quiet dependency flag didn't silence warnings in some
Expand Down
3 changes: 1 addition & 2 deletions lib/src/extend/extension_store.dart
Expand Up @@ -262,8 +262,7 @@ class ExtensionStore {
_extensionsByExtender.putIfAbsent(simple, () => []).add(extension);
// Only source specificity for the original selector is relevant.
// Selectors generated by `@extend` don't get new specificity.
_sourceSpecificity.putIfAbsent(
simple, () => complex.maxSpecificity);
_sourceSpecificity.putIfAbsent(simple, () => complex.maxSpecificity);
}

if (selectors != null || existingExtensions != null) {
Expand Down
8 changes: 4 additions & 4 deletions lib/src/visitor/async_evaluate.dart
Expand Up @@ -592,7 +592,7 @@ class _EvaluateVisitor
/// The [stackFrame] and [nodeWithSpan] are used for the name and location of
/// the stack frame for the duration of the [callback].
Future<void> _loadModule(Uri url, String stackFrame, AstNode nodeWithSpan,
void callback(Module module),
FutureOr<void> callback(Module module),
{Uri? baseUrl,
Configuration? configuration,
bool namesInErrors = false}) async {
Expand All @@ -606,7 +606,7 @@ class _EvaluateVisitor
configuration.nodeWithSpan.span);
}

_addExceptionSpan(nodeWithSpan, () => callback(builtInModule));
await _addExceptionSpanAsync(nodeWithSpan, () => callback(builtInModule));
return;
}

Expand Down Expand Up @@ -643,7 +643,7 @@ class _EvaluateVisitor
}

try {
callback(module);
await callback(module);
} on SassRuntimeException {
rethrow;
} on MultiSpanSassException catch (error) {
Expand Down Expand Up @@ -3212,7 +3212,7 @@ class _EvaluateVisitor

/// Like [_addExceptionSpan], but for an asynchronous [callback].
Future<T> _addExceptionSpanAsync<T>(
AstNode nodeWithSpan, Future<T> callback()) async {
AstNode nodeWithSpan, FutureOr<T> callback()) async {
try {
return await callback();
} on MultiSpanSassScriptException catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/visitor/evaluate.dart
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 1249b1184a46950409776772cb1c6003b80ebb31
// Checksum: b36c6c618b222a025ed67c1df8afb66e037a132b
//
// ignore_for_file: unused_import

Expand Down
12 changes: 12 additions & 0 deletions test/dart_api_test.dart
Expand Up @@ -225,4 +225,16 @@ a {
});
});
});

// Regression test for #1318
test("meta.load-module() doesn't have a race condition", () async {
await d.file("other.scss", '/**//**/').create();
expect(compileStringAsync("""
@use 'sass:meta';
@include meta.load-css("other.scss");
""", loadPaths: [d.sandbox]), completion(equals("/**/\n/**/")));

// Give the race condition time to appear.
await pumpEventQueue();
});
}

0 comments on commit 83343d7

Please sign in to comment.