Opened 8 years ago

Closed 8 years ago

#15137 closed defect (duplicate)

race condition referencing circular dependency from plugin

Reported by: bill Owned by: Rawld Gill
Priority: low Milestone: 1.8
Component: Loader Version: 1.7.2
Keywords: Cc: Adam Peller
Blocked By: Blocking:

Description (last modified by bill)

This is the intermittent Maqetta failure that Adam mentioned in #13769, triggered by the circular dependency between dojo/dom-construct.js and dojo/dom-attr.js:

  1. On Maquetta startup, some module calls the css.js plugin as: davinci/css!davinci/review/resources/Comment.css
  1. css.js depends on dojo/dom-construct. Before the css.js plugin runs, dom-construct.js has been loaded, and it's factory has been run. In other words, the construct variable in css.js contains the methods defined in dom-construct.js.
  1. However, dojo/dom-construct has a dependency on dojo/dom-attr, and although dom-attr.js is already loaded, it's factory code hasn't been run yet, so require("dojo/dom-attr") === {}.
  1. Thus the css.js plugin runs, calls construct.create(), which tries to call attr.set() (or something like that), and fails, since the dom-attr.js factory hasn't been run yet.

The failure is complicated to recreate because you need to download and install maquetta, as per the instructions in https://github.com/maqetta/maqetta/wiki/Developer-Setup, and then comment out the workaround code added in https://github.com/maqetta/maqetta/issues/2047. I tried reducing it to a simpler case but no luck.

I thought you (Rawld) might be able to surmise the problem though given this description, without actually installing maquetta. Maquetta is using the sync loader, and this failure is happening against source (as opposed to a build).

Alternately, if this pattern isn't supposed to work, just let us know. Obviously the dom-construct.js factory shouldn't try to call functions in dom-attr.js, and vice-versa, but is it also true that other module factories shouldn't call functions in dom-attr.js and dom-construct.js? And by corollary that plugins shouldn't call methods in dom-attr.js and dom-construct.js? If so, the circular dependency just needs to be broken.

Attachments (1)

circle-with-plugin.zip (2.0 KB) - added by Rawld Gill 8 years ago.
test case with plugin with circular dependency

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by bill

Cc: Adam Peller added
Owner: changed from Rawld Gill to bill
Status: newassigned

comment:2 Changed 8 years ago by bill

Description: modified (diff)

comment:3 Changed 8 years ago by bill

Description: modified (diff)

comment:4 Changed 8 years ago by bill

Summary: race condition referencing indirect dependency when circular dependency existsrace condition referencing circular dependency from plugin

comment:5 Changed 8 years ago by Adam Peller

Maqetta is using the asynch loader

comment:6 Changed 8 years ago by bill

Owner: changed from bill to Rawld Gill

I stand corrected.

Accidentally assigned this to myself, meant to give it to Rawld. Although again, I'm not sure if it's a bug or just unsupported. It's certainly a corner case, so I'm fine to say that it's not supported.

Changed 8 years ago by Rawld Gill

Attachment: circle-with-plugin.zip added

test case with plugin with circular dependency

comment:7 Changed 8 years ago by Rawld Gill

Milestone: tbdfuture
Priority: undecidedlow

If I understand Bill's description, this should work correctly (assuming a rational build system like Dojo's). fwiw, I created and attached a test case that has a plugin that depends on dojo/dom-construct together with a build that puts all the req'd modules in a single layer (the dojo layer). To exercise the test case, unzip and place the tree as a sibling of dojo, run the build like this

path/to/dtk/util/buildscripts:./build.sh -p ../../circle-with-plugin/circle-with-plugin -r

and then load

path/to/dtk/circle-with-plugin/test.html

in the browser. I tried all kinds of combinations of loading dom-construct/attr before the plugin and it always worked perfectly.

It concerns me that maqetta seems to have an intermittent failure mode. If would be great if this could somehow be reduced. I recommend building with the switches

--layerOptimize 0 --optimize 0

and with "dojo-trace-api" and "dojo-publish-privates" has features set static true. Then turn tracing circular dependencies on. The easiest way is to just set the flag at the bottom of the built dojo. You can also set via normal config, e.g.,

require({trace:{
"loader-circular-dependency":1
}});

Lastly, the apparent work-around--to put a direct, seemingly-unnecessary dependency on dojo/dom-attr in the plugin--is a good one (kind of a belt and suspenders approach). Can anybody confirm that hack is working.

I'm going to lower the priority of this until I get more feedback.

comment:8 Changed 8 years ago by bill

IIRC the problem reproduced against source, not a build, but yeah with your simple testcase (and the one I tried) the problem doesn't reproduce against source or build. It does happen w/the instructions I listed above.

comment:9 Changed 8 years ago by Rawld Gill

I believe I found a likely suspect to this problem and fixed in [28456] + [28457]; see #15061.

It would be great if bill or adam could pull trunk and see if this problem has magically disappeared.

comment:10 Changed 8 years ago by Rawld Gill

Milestone: future1.8
Resolution: duplicate
Status: assignedclosed

Adam Peller tested trunk against Maqetta and was unable to reproduce the problem. I believe this is a duplicate problem fixed in [28456] (in particular, the change at line 1271).

Note: See TracTickets for help on using tickets.