Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inline and less imports of the same name = race condition #3170

Merged
merged 1 commit into from
Feb 19, 2018
Merged

inline and less imports of the same name = race condition #3170

merged 1 commit into from
Feb 19, 2018

Conversation

thorn0
Copy link
Contributor

@thorn0 thorn0 commented Feb 19, 2018

Stumbled upon this working on webpack-contrib/less-loader#242.
See an instance of the mentioned race condition here: https://ci.appveyor.com/project/webpack-contrib/less-loader/build/1.0.46/job/mbywf90cimqkjee5

It happens when processing files like this:

@import (less) "some/css.css";
@import (inline) "some/css.css";

…ndition

Stumbled upon this working on webpack-contrib/less-loader#242.
See an instance of the mentioned race condition here: https://ci.appveyor.com/project/webpack-contrib/less-loader/build/1.0.46/job/mbywf90cimqkjee5

It happens when processing files like this:
```less
@import (less) "some/css.css";
@import (inline) "some/css.css";
```
@thorn0 thorn0 changed the title Having inline and less imports of the same name lead to a race condition inline and less imports of the same name = race condition Feb 19, 2018
@seven-phases-max
Copy link
Member

seven-phases-max commented Feb 19, 2018

I'm not sure I actually understand the issue... Could you elaborate please?
Is this about once/multiple?

@thorn0
Copy link
Contributor Author

thorn0 commented Feb 19, 2018

It's about less vs inline. Processed imports are cached in files by their paths. However, inline issues aren't parsed. Instead their root property contains an unparsed string. If two imports of the same name start being processed and the inline one gets processed first, the less one isn't parsed. An entry created for the inline one is retrieved from files and things go wrong. Code gets a string where an object is expected. My patch prevents inline imports from storing in files. They aren't reused from there anyway.

A better solution would be not to use only path as a key for files, queue, and probably other places and take import options into account too. BTW, is this queue property actually used anywhere?

@seven-phases-max
Copy link
Member

seven-phases-max commented Feb 19, 2018

I see. So... is anything of these: 1, 2, 3 is changed with this patch (or not working in 3.x w/o it)?
If there's difference we need to know if something should be reflected in the docs.

@thorn0
Copy link
Contributor Author

thorn0 commented Feb 19, 2018

(1) and (2) sometimes don't work (an exception is thrown) without it. That's it.

@seven-phases-max
Copy link
Member

I see. So the issue is a sort of #1898 replica.

@seven-phases-max seven-phases-max merged commit 3699921 into less:master Feb 19, 2018
@seven-phases-max
Copy link
Member

Thanks.

@thorn0 thorn0 deleted the patch-1 branch February 19, 2018 13:39
@matthew-dean
Copy link
Member

Sounds like a bug I may have introduced. I added caching to files in 3.x. Good catch.

@thorn0
Copy link
Contributor Author

thorn0 commented Feb 23, 2018

Can we possibly have a 3.0.2 release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants