mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 05:51:39 +00:00
Fix server crash when deleting folders
Handle the case where a folder's parent path no longer exists when processing VFS removal events. This can happen when a folder and its contents are deleted together, causing a race between removal events. Added a regression test for this scenario.
This commit is contained in:
parent
4965165ad5
commit
752be0d56f
8 changed files with 137 additions and 4 deletions
|
|
@ -0,0 +1,16 @@
|
||||||
|
---
|
||||||
|
source: tests/tests/serve.rs
|
||||||
|
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
|
||||||
|
---
|
||||||
|
instances:
|
||||||
|
id-2:
|
||||||
|
Children: []
|
||||||
|
ClassName: Folder
|
||||||
|
Id: id-2
|
||||||
|
Metadata:
|
||||||
|
ignoreUnknownInstances: false
|
||||||
|
Name: remove_folder
|
||||||
|
Parent: "00000000000000000000000000000000"
|
||||||
|
Properties: {}
|
||||||
|
messageCursor: 1
|
||||||
|
sessionId: id-1
|
||||||
|
|
@ -0,0 +1,38 @@
|
||||||
|
---
|
||||||
|
source: tests/tests/serve.rs
|
||||||
|
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
|
||||||
|
---
|
||||||
|
instances:
|
||||||
|
id-2:
|
||||||
|
Children:
|
||||||
|
- id-3
|
||||||
|
ClassName: Folder
|
||||||
|
Id: id-2
|
||||||
|
Metadata:
|
||||||
|
ignoreUnknownInstances: false
|
||||||
|
Name: remove_folder
|
||||||
|
Parent: "00000000000000000000000000000000"
|
||||||
|
Properties: {}
|
||||||
|
id-3:
|
||||||
|
Children:
|
||||||
|
- id-4
|
||||||
|
ClassName: Folder
|
||||||
|
Id: id-3
|
||||||
|
Metadata:
|
||||||
|
ignoreUnknownInstances: false
|
||||||
|
Name: my-folder
|
||||||
|
Parent: id-2
|
||||||
|
Properties: {}
|
||||||
|
id-4:
|
||||||
|
Children: []
|
||||||
|
ClassName: StringValue
|
||||||
|
Id: id-4
|
||||||
|
Metadata:
|
||||||
|
ignoreUnknownInstances: false
|
||||||
|
Name: test
|
||||||
|
Parent: id-3
|
||||||
|
Properties:
|
||||||
|
Value:
|
||||||
|
String: "This file is inside my-folder\r\n"
|
||||||
|
messageCursor: 0
|
||||||
|
sessionId: id-1
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
---
|
||||||
|
source: tests/tests/serve.rs
|
||||||
|
expression: redactions.redacted_yaml(info)
|
||||||
|
---
|
||||||
|
expectedPlaceIds: ~
|
||||||
|
gameId: ~
|
||||||
|
placeId: ~
|
||||||
|
projectName: remove_folder
|
||||||
|
protocolVersion: 5
|
||||||
|
rootInstanceId: id-2
|
||||||
|
serverVersion: "[server-version]"
|
||||||
|
sessionId: id-1
|
||||||
|
unexpectedPlaceIds: ~
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
---
|
||||||
|
source: tests/tests/serve.rs
|
||||||
|
expression: "socket_packet.intern_and_redact(&mut redactions, ())"
|
||||||
|
---
|
||||||
|
body:
|
||||||
|
messageCursor: 1
|
||||||
|
messages:
|
||||||
|
- added: {}
|
||||||
|
removed:
|
||||||
|
- id-3
|
||||||
|
updated: []
|
||||||
|
packetType: messages
|
||||||
|
sessionId: id-1
|
||||||
6
rojo-test/serve-tests/remove_folder/default.project.json
Normal file
6
rojo-test/serve-tests/remove_folder/default.project.json
Normal file
|
|
@ -0,0 +1,6 @@
|
||||||
|
{
|
||||||
|
"name": "remove_folder",
|
||||||
|
"tree": {
|
||||||
|
"$path": "src"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1 @@
|
||||||
|
This file is inside my-folder
|
||||||
|
|
@ -172,12 +172,23 @@ impl JobThreadContext {
|
||||||
self.apply_patches(self.vfs.canonicalize(&path).unwrap())
|
self.apply_patches(self.vfs.canonicalize(&path).unwrap())
|
||||||
}
|
}
|
||||||
VfsEvent::Remove(path) => {
|
VfsEvent::Remove(path) => {
|
||||||
// MemoFS does not track parent removals yet, so we can canonicalize
|
// Try to canonicalize the parent path and append the removed path's
|
||||||
// the parent path safely and then append the removed path's file name.
|
// file name. If the parent no longer exists (e.g., a folder and its
|
||||||
|
// contents were deleted together), we skip processing since the
|
||||||
|
// parent's removal event will handle cleaning up the instances.
|
||||||
let parent = path.parent().unwrap();
|
let parent = path.parent().unwrap();
|
||||||
let file_name = path.file_name().unwrap();
|
let file_name = path.file_name().unwrap();
|
||||||
let parent_normalized = self.vfs.canonicalize(parent).unwrap();
|
match self.vfs.canonicalize(parent) {
|
||||||
self.apply_patches(parent_normalized.join(file_name))
|
Ok(parent_normalized) => self.apply_patches(parent_normalized.join(file_name)),
|
||||||
|
Err(err) => {
|
||||||
|
log::debug!(
|
||||||
|
"Parent path no longer exists for removed path {}: {}",
|
||||||
|
path.display(),
|
||||||
|
err
|
||||||
|
);
|
||||||
|
Vec::new()
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
_ => {
|
_ => {
|
||||||
log::warn!("Unhandled VFS event: {:?}", event);
|
log::warn!("Unhandled VFS event: {:?}", event);
|
||||||
|
|
|
||||||
|
|
@ -126,6 +126,41 @@ fn remove_file() {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Test that removing a folder (directory) with contents doesn't crash the server.
|
||||||
|
/// This is a regression test for a bug where deleting a folder would cause a panic
|
||||||
|
/// due to the parent path no longer existing when processing child removal events.
|
||||||
|
#[test]
|
||||||
|
fn remove_folder() {
|
||||||
|
run_serve_test("remove_folder", |session, mut redactions| {
|
||||||
|
let info = session.get_api_rojo().unwrap();
|
||||||
|
let root_id = info.root_instance_id;
|
||||||
|
|
||||||
|
assert_yaml_snapshot!("remove_folder_info", redactions.redacted_yaml(info));
|
||||||
|
|
||||||
|
let read_response = session.get_api_read(root_id).unwrap();
|
||||||
|
assert_yaml_snapshot!(
|
||||||
|
"remove_folder_all",
|
||||||
|
read_response.intern_and_redact(&mut redactions, root_id)
|
||||||
|
);
|
||||||
|
|
||||||
|
fs::remove_dir_all(session.path().join("src/my-folder")).unwrap();
|
||||||
|
|
||||||
|
let socket_packet = session
|
||||||
|
.get_api_socket_packet(SocketPacketType::Messages, 0)
|
||||||
|
.unwrap();
|
||||||
|
assert_yaml_snapshot!(
|
||||||
|
"remove_folder_subscribe",
|
||||||
|
socket_packet.intern_and_redact(&mut redactions, ())
|
||||||
|
);
|
||||||
|
|
||||||
|
let read_response = session.get_api_read(root_id).unwrap();
|
||||||
|
assert_yaml_snapshot!(
|
||||||
|
"remove_folder_all-2",
|
||||||
|
read_response.intern_and_redact(&mut redactions, root_id)
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn edit_init() {
|
fn edit_init() {
|
||||||
run_serve_test("edit_init", |session, mut redactions| {
|
run_serve_test("edit_init", |session, mut redactions| {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue