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.

Sign up for GitHub

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

fix: calling setMode just before destroy causes error reading getLength #5685

Closed
wants to merge 2 commits into from
Closed

fix: calling setMode just before destroy causes error reading getLength #5685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e8856e
fix: calling setMode just before destroy causes error reading getLength
nlujjawal Nov 21, 2024
1747f65
adding test case
nlujjawal Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/edit_session.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,9 @@ class EditSession {
// load on demand
this.$modeId = path;
config.loadModule(["mode", path], function(m) {
if (this.destroyed) {
return;
}
if (this.$modeId !== path)
return cb && cb();
if (this.$modes[path] && !options) {
Expand Down
36 changes: 36 additions & 0 deletions src/edit_session_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,42 @@ module.exports = {
});
}, 0);
},
"test: module loading callback is aborted when session is destroyed": function(next) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simply uncomment session.destroy(); on line 1151 above instead of adding a new test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in that case removing the code for the fix as well makes test case run successfully.

// if (!require.undef) {
// console.log("Skipping test: This test only runs in the browser");
// next();
// return;
// }
console.log("Not Skipping test: This test only runs in the browser");

var session = new EditSession([]);
var onChangeModeCallCount = 0;
var changeModeCallbackCallCount = 0;
var originalOnChangeMode = session.$onChangeMode;

// Create spy
session.$onChangeMode = function(...arguments) {
onChangeModeCallCount++;
originalOnChangeMode.apply(this, arguments);
};

// Start setting mode
session.setMode("ace/mode/javascript", function(mode) {
changeModeCallbackCallCount++;
});

// Destroy session before mode loading completes
session.destroy();

// Give some time for any pending operations to complete
setTimeout(function() {
assert.equal(onChangeModeCallCount, 0, "Mode change event should not fire after destruction");
assert.equal(changeModeCallbackCallCount, 0, "Mode change callback should not fire after destruction");
assert.equal(session.destroyed, true, "Session should remain destroyed");
assert.equal(session.$modeid, undefined, "Mode ID should not be set after destruction");
next();
}, 50);
},

"test: sets destroyed flag when destroy called and tokenizer is never null": function() {
var session = new EditSession(["foo bar foo bar"]);
Expand Down
Loading