Opened 4 years ago
Closed 4 years ago
#18637 closed defect (fixed)
aspect.before and aspect.after can cause errors if multiple aspect handles are removed within a handler
Reported by: | Kenneth G. Franqueiro | Owned by: | Kenneth G. Franqueiro |
---|---|---|---|
Priority: | undecided | Milestone: | 1.8.11 |
Component: | Core | Version: | 1.10.4 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
This is probably easiest to explain with a couple of examples.
var handle1 = aspect.before(obj, 'method', function () { // ... }); var handle2 = aspect.before(obj, 'method', function () { // ... handle2.remove(); handle1.remove(); }); obj.method(); // throws error
var handle1 = aspect.after(obj, 'method', function () { handle1.remove(); handle2.remove(); // ... }); var handle2 = aspect.after(obj, 'method', function () { // ... }); obj.method(); // throws error
If you reverse the order in which the handles are removed, the error does not occur.
I believe what's happening is that in these cases, when you remove the handles in this order, the handle from the first removed aspect still maintains a reference to the second removed aspect, and it still attempts to run that as a result when the aspect code navigates to next
. When removed in the reverse order, all of the next
and previous
references that need to be updated end up properly updated.
This can be prevented with a couple of checks in the respective code paths, since the error that occurs is always due to the advice
function being null. I'll be submitting a PR against master shortly, including tests.
Backporting this will require some additional work to backport the tests to the DOH modules.
Change History (3)
comment:1 Changed 4 years ago by
Milestone: | tbd → 1.11 |
---|---|
Owner: | set to Kenneth G. Franqueiro |
Status: | new → assigned |
comment:2 Changed 4 years ago by
Milestone: | 1.11 → 1.10.5 |
---|
comment:3 Changed 4 years ago by
Milestone: | 1.10.5 → 1.8.11 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Backported:
- 1.10: https://github.com/dojo/dojo/commit/2bcaa187b072c07bc9de59c1a9d2930226f4bc5c
- 1.9: https://github.com/dojo/dojo/commit/505b200d057259f2e39f03444136f22592204bd1
- 1.8: https://github.com/dojo/dojo/commit/d702821f9dc3230186bccd8b1df249b1f0ed682f
This fix would appear in 1.8.11+, 1.9.8+, and 1.10.5+.
Merged to master via https://github.com/dojo/dojo/pull/160. Needs backports.