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

Race Condition #1898

Closed
ForbesLindesay opened this issue Feb 25, 2014 · 10 comments
Closed

Race Condition #1898

ForbesLindesay opened this issue Feb 25, 2014 · 10 comments
Assignees

Comments

@ForbesLindesay
Copy link
Contributor

I've found what I think is a race condition in less. Unfortunately the minimum example I can come up with involves 5 files. With the lessc command line it then outputs one of two different things at random. If you add the sync-imports flag, the problem is mitigated.

Inputs

index.less

@import "indirect-a.less";
@import "indirect-b.less";

body {
  color: @color;
}

indirect-a.less

@import "variables.less";
@import "override.less";

indirect-b.less

@import "variables.less";

variables.less

@color: red;

override.less

@color: black;

Command

lessc index.less

Output

One of the following two options is output at random:

Aprox 95% of the time:

body {
  color: #000000;
}

Aprox 5% of the time:

body {
  color: #ff0000;
}

This occurs with the latest version in npm and with the version currently in the GitHub repo.

@lukeapage
Copy link
Member

cool many thanks for the repro case, I'll take a look

@lukeapage lukeapage self-assigned this Feb 25, 2014
@lukeapage
Copy link
Member

I've changed import-once to work out duplicates in a consistent way rather than relying on always taking the first file load that returns

@ForbesLindesay
Copy link
Contributor Author

Thanks

@pmonegan
Copy link

Because I haven't tried it yet, the fixed outcome should be red?
On Feb 27, 2014 12:14 AM, "Luke Page" [email protected] wrote:

I've changed import-once to work out duplicates in a consistent way rather
than relying on always taking the first file load that returns

Reply to this email directly or view it on GitHubhttps://github.com//issues/1898#issuecomment-36213828
.

@seven-phases-max
Copy link
Member

@pmonegan
Nope, it's actually black. The tricky part is that by default the @import directive implies once option, so the last occurrence of variables.less (set in indirect-b.less) is not expanded hence the latest definition is @color: black; (because the last imported file is override.less).

It will become red if you explicitly specify multiple flag for either indirect-b.less or for variables.less inside it, e.g.:

@import "indirect-a.less";
@import (multiple) "indirect-b.less";

body {
  color: @color;
}

@pmonegan
Copy link

Is this based only on node.js non - blocking, or as well in browser.
On Feb 28, 2014 12:22 PM, "Max Mikhailov" [email protected] wrote:

@pmonegan https://github.com/pmonegan
Nope, it's actually black. The tricky part is that by default the @importdirective implies
once http://lesscss.org/features/#import-options-once option, so the
last occurrence of "variables.less" (set in "indirect-b.less") is not
expanded hence the latest definition is @color: black; (because the last
imported file is "override.less").

Reply to this email directly or view it on GitHubhttps://github.com//issues/1898#issuecomment-36379075
.

@seven-phases-max
Copy link
Member

I did not test in a browser, but it should be the same (and should not be affected by blocking/non-blocking stuff, unless there're some other hidden bugs of course).

@pmonegan
Copy link

Ok. I'll let you know if you haven't tested it by then.
On Feb 28, 2014 12:31 PM, "Max Mikhailov" [email protected] wrote:

I did not test in a browser, but it should be the same (and not depend on
blocking/non-blocking, unless there're some other hidden bugs).

Reply to this email directly or view it on GitHubhttps://github.com//issues/1898#issuecomment-36379973
.

@lukeapage
Copy link
Member

It doesn't block loading files it just changes when it decides on the
duplicate.

@pmonegan
Copy link

Currently out of 100 browser refresh tests (FF, IE11) , @import is doing
what it's supposed to do. Same code returned @color: black each time.

On Fri, Feb 28, 2014 at 12:38 PM, Luke Page [email protected]:

It doesn't block loading files it just changes when it decides on the
duplicate.

Reply to this email directly or view it on GitHubhttps://github.com//issues/1898#issuecomment-36380649
.

tobiasso85 added a commit to SAP/less-openui5 that referenced this issue Nov 12, 2019
In less 1.6.3 there is an issue with the same imports across different
nested imports. E.g. if the same import is used in 2 different less
files the order of loading is not guaranteed.
To ensure reproducibility of this issue the build is run 50 times.

refs: less/less.js#1898
matz3 pushed a commit to SAP/less-openui5 that referenced this issue Nov 14, 2019
In less 1.6.3 there is an issue with the same imports across different
nested imports. E.g. if the same import is used in 2 different less
files the order of loading is not guaranteed.
To ensure reproducibility of this issue the build is run 50 times.

refs: less/less.js#1898
matz3 pushed a commit to SAP/less-openui5 that referenced this issue Nov 14, 2019
In less 1.6.3 there is an issue with the same imports across different
nested imports. E.g. if the same import is used in 2 different less
files the order of loading is not guaranteed.
To ensure reproducibility of this issue the build is run 50 times.

refs: less/less.js#1898
matz3 pushed a commit to SAP/less-openui5 that referenced this issue Nov 14, 2019
In less 1.6.3 there is an issue with the same imports across different
nested imports. E.g. if the same import is used in 2 different less
files the order of loading is not guaranteed.
To ensure reproducibility of this issue the build is run 50 times.

refs: less/less.js#1898
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants