Skip to content

Commit 4c5fcfe

Browse files
Titanium2099Titanium2099
committed
fix: Use transactions to prevent event interleaving race condition
1 parent 6682f53 commit 4c5fcfe

File tree

3 files changed

+84
-114
lines changed

3 files changed

+84
-114
lines changed

‎src/addons/addons/collaboration/helpers/collaboration-ui.js‎

Lines changed: 65 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -953,149 +953,115 @@ export function setUndoRedoOverride() {
953953
}, 1000); // Reset after 1 second (this timeout length might need tuning for reliability).
954954
}
955955

956+
/**
957+
* Flushes the event buffer for a given group ID, sending all buffered events
958+
* as a single atomic transaction.
959+
* @param {string} groupId The ID of the event group to flush.
960+
*/
961+
function flushEventBuffer(groupId) {
962+
const buffer = constants.mutableRefs.eventTransactionBuffer;
963+
if (buffer.has(groupId)) {
964+
const eventBatch = buffer.get(groupId);
965+
if (eventBatch.length > 0) {
966+
constants.mutableRefs.ydoc.transact(() => {
967+
constants.mutableRefs.yEvents.push([eventBatch]);
968+
if (constants.debugging) console.log(`Collab Send: Flushed ${eventBatch.length} events for group ${groupId}.`);
969+
}, constants.LOCAL_EVENT_SYNC_ORIGIN);
970+
}
971+
buffer.delete(groupId);
972+
}
973+
}
974+
956975
/**
957976
* Central handler for all Blockly/VM events that need to be broadcast for collaboration.
958-
* This function filters out non-relevant events and prepares the payload
959-
* before pushing it to the Yjs shared event array (`constants.mutableRefs.yEvents`).
977+
* This function filters out non-relevant events, prepares the payload, and then
978+
* either sends it immediately (for isolated events) or buffers it to be sent
979+
* in a transaction with other events from the same user action.
960980
* @param {object} event - The Blockly event object.
961981
*/
962982
export function handleBlocklyEventForCollaboration(event) {
963-
// console.log("Blockly Event:", event.type, event); // Debug log for all events.
983+
if (!constants.mutableRefs.BlocklyInstance || !constants.mutableRefs.BlocklyInstance.Events.isEnabled()) return;
964984

965-
// Special handling for 'dragOutside' events: set a flag.
966985
if (event.type === 'dragOutside') {
967986
constants.mutableRefs.dragOutsideStarted = true;
968-
return; // Do not process 'dragOutside' as a regular event.
987+
return;
969988
}
970989

971-
// Skip if Blockly events are globally disabled (e.g., during remote sync).
972-
if (!constants.mutableRefs.BlocklyInstance || !constants.mutableRefs.BlocklyInstance.Events.isEnabled()) return;
973-
974990
const workspace = constants.mutableRefs.BlocklyInstance.getMainWorkspace();
975-
// // Optional: Ensure event is for the main workspace. Currently commented out.
976-
// if (!workspace || event.workspaceId !== workspace.id) return;
977-
978-
// Filter out UI-only events that do not represent actual model changes.
979-
// `event.recordUndo` is a good proxy for events that modify the project state.
980-
// Explicitly allow events related to drag-outside (delete/move) or backpack/undo/redo overrides.
981-
const isDragOutsideMoveOrDelete = constants.mutableRefs.dragOutsideStarted &&
982-
["move", "delete"].includes(event.type);
983-
984-
const isBackpackOverride = backpackInsertOverride &&
985-
["move", "create"].includes(event.type);
986991

992+
const isDragOutsideMoveOrDelete = constants.mutableRefs.dragOutsideStarted && ["move", "delete"].includes(event.type);
993+
const isBackpackOverride = backpackInsertOverride && ["move", "create"].includes(event.type);
987994
const isUndoRedoOverride = (undoRedoOverride && ["create", "move", "delete", "var_create"].includes(event.type)) ||
988-
(event.type === 'var_rename' && event.varId && event.oldName && event.newName); // var_rename might not have recordUndo but needs sync.
995+
(event.type === 'var_rename' && event.varId && event.oldName && event.newName);
989996

990-
// If an event is not undoable AND not covered by the overrides, skip it.
991997
if (!event.recordUndo && !(isDragOutsideMoveOrDelete || isBackpackOverride || isUndoRedoOverride)) {
992-
// console.log("Collab Send: Skipping non-undoable event:", event.type, event.element || '', event); // Debug log.
993998
return;
994999
} else if (!event.recordUndo && constants.mutableRefs.dragOutsideStarted) {
995-
// Reset dragOutsideStarted flag if it was set and this event was part of a drag-outside sequence.
9961000
constants.mutableRefs.dragOutsideStarted = false;
9971001
}
9981002

999-
// Filter out events for blocks within the flyout (toolbox) or mutator, as these are temporary UI elements.
10001003
if (event.type === constants.mutableRefs.BlocklyInstance.Events.CREATE) {
10011004
const block = workspace.getBlockById(event.blockId);
10021005
if (block?.isInFlyout || block?.isInMutator) return;
10031006
}
10041007

1005-
// --- Determine Editing Target Context ---
1006-
const targetName = helper.getCurrentEditingTargetName(); // Get the name of the currently edited target.
1007-
let targetId = null;
1008-
const editingTarget = constants.mutableRefs.vm.runtime.getEditingTarget();
1009-
if (editingTarget) {
1010-
targetId = editingTarget.id;
1011-
}
1012-
1008+
const targetName = helper.getCurrentEditingTargetName();
10131009
let eventJson = null;
10141010

1015-
// --- Prepare Payload for Variable Events ---
10161011
if (event.type === constants.mutableRefs.BlocklyInstance.Events.VAR_CREATE) {
1017-
if (typeof event.varId !== 'string' || typeof event.varName !== 'string') {
1018-
console.warn("Collab Send [VarCreate]: Invalid variable ID or name:", event);
1019-
return;
1020-
}
1021-
// Determine the target name for local variables, or null for global.
1012+
if (typeof event.varId !== 'string' || typeof event.varName !== 'string') return;
10221013
const varTargetName = event.isLocal ? (constants.mutableRefs.vm.runtime.getEditingTarget()?.getName() || targetName) : null;
1023-
if (event.isLocal && !varTargetName) {
1024-
console.warn("Collab Send [VarCreate]: Local variable event but could not get target name. Skipping.", event);
1025-
return;
1026-
}
1027-
eventJson = {
1028-
type: "var_create",
1029-
isCloud: event.isCloud,
1030-
isLocal: event.isLocal,
1031-
id: event.varId, // Send original ID as a hint for receiver.
1032-
name: event.varName,
1033-
varType: event.varType,
1034-
targetName: varTargetName // Name of the target sprite if local, else null.
1035-
};
1014+
if (event.isLocal && !varTargetName) return;
1015+
eventJson = { type: "var_create", isCloud: event.isCloud, isLocal: event.isLocal, id: event.varId, name: event.varName, varType: event.varType, targetName: varTargetName };
10361016
} else if (event.type === constants.mutableRefs.BlocklyInstance.Events.VAR_RENAME) {
1037-
if (typeof event.varId !== 'string' || typeof event.oldName !== 'string' || typeof event.newName !== 'string') {
1038-
console.warn("Collab Send [VarRename]: Invalid data for rename:", event);
1039-
return;
1040-
}
1041-
eventJson = {
1042-
type: "var_rename",
1043-
originalVarId: event.varId,
1044-
oldName: event.oldName,
1045-
newName: event.newName,
1046-
ambiguityFix: { // Provide additional context to help receiver identify the variable uniquely.
1047-
varType: event.variable.type,
1048-
isCloud: event.variable.isCloud,
1049-
isLocal: event.variable.isLocal,
1050-
}
1051-
};
1017+
if (typeof event.varId !== 'string' || typeof event.oldName !== 'string' || typeof event.newName !== 'string') return;
1018+
eventJson = { type: "var_rename", originalVarId: event.varId, oldName: event.oldName, newName: event.newName, ambiguityFix: { varType: event.variable.type, isCloud: event.variable.isCloud, isLocal: event.variable.isLocal } };
10521019
} else if (event.type === constants.mutableRefs.BlocklyInstance.Events.VAR_DELETE) {
1053-
if (typeof event.varId !== 'string' || typeof event.varName !== 'string') {
1054-
console.warn("Collab Send [VarDelete]: Invalid data for delete:", event);
1055-
return;
1056-
}
1057-
eventJson = {
1058-
type: "var_delete",
1059-
originalVarId: event.varId,
1060-
varName: event.varName,
1061-
ambiguityFix: { // Provide additional context to help receiver identify the variable uniquely.
1062-
varType: event.varType,
1063-
isCloud: event.isCloud,
1064-
isLocal: event.isLocal,
1065-
}
1066-
};
1020+
if (typeof event.varId !== 'string' || typeof event.varName !== 'string') return;
1021+
eventJson = { type: "var_delete", originalVarId: event.varId, varName: event.varName, ambiguityFix: { varType: event.varType, isCloud: event.isCloud, isLocal: event.isLocal } };
10671022
}
10681023

1069-
// If it's not a variable event, convert the Blockly event to JSON.
10701024
if (eventJson == null) {
10711025
try {
10721026
eventJson = event.toJson();
1073-
// Additional check for variable events that might pass through this general `toJson` path
1074-
// (e.g., if a new variable event type is introduced).
1075-
if (event.type.startsWith('var_') && event.type !== constants.mutableRefs.BlocklyInstance.Events.VAR_RENAME && (!event.varId || typeof event.varName === 'undefined')) {
1076-
console.warn("Collab Send: Variable event missing ID or Name, skipping:", event);
1077-
return;
1078-
}
1027+
if (event.type.startsWith('var_') && event.type !== constants.mutableRefs.BlocklyInstance.Events.VAR_RENAME && (!event.varId || typeof event.varName === 'undefined')) return;
10791028
} catch (e) {
10801029
console.error("Collab Send: Error serializing event:", e, event);
10811030
return;
10821031
}
10831032
}
10841033

1085-
// --- Broadcast the Event via Yjs ---
1086-
if (constants.mutableRefs.yEvents && constants.mutableRefs.ydoc) {
1087-
if (constants.debugging) console.log(`Collab Send: Event=${event.type} Target=${targetName} Block=${event.blockId || 'N/A'} VarId=${event.varId || 'N/A'}`);
1088-
// Wrap the event JSON and target context, then push to the shared Yjs array.
1089-
// `constants.LOCAL_EVENT_SYNC_ORIGIN` is used to mark this as a local event.
1034+
if (!constants.mutableRefs.yEvents || !constants.mutableRefs.ydoc) {
1035+
console.warn("Collab Send: yEvents or ydoc not ready. Event not sent.");
1036+
return;
1037+
}
1038+
1039+
const eventPackage = {
1040+
targetName: targetName,
1041+
event: eventJson,
1042+
timestamp: Date.now()
1043+
};
1044+
1045+
const groupId = Blockly.Events.getGroup();
1046+
1047+
if (!groupId) {
1048+
// This is an isolated event (not part of a drag, etc.). Send it immediately in its own transaction.
10901049
constants.mutableRefs.ydoc.transact(() => {
1091-
constants.mutableRefs.yEvents.push([{
1092-
targetName: targetName, // Context for receiver.
1093-
event: eventJson,
1094-
timestamp: Date.now() // Add timestamp for chronological processing.
1095-
}]);
1050+
constants.mutableRefs.yEvents.push([[eventPackage]]); // Wrap in an array to mark it as a batch of one.
1051+
if (constants.debugging) console.log(`Collab Send: Event=${event.type} sent immediately.`);
10961052
}, constants.LOCAL_EVENT_SYNC_ORIGIN);
10971053
} else {
1098-
console.warn("Collab Send: yEvents or ydoc not ready. Event not sent.");
1054+
// This event is part of a group. Buffer it.
1055+
const buffer = constants.mutableRefs.eventTransactionBuffer;
1056+
clearTimeout(constants.mutableRefs.eventFlushTimer);
1057+
1058+
if (!buffer.has(groupId)) {
1059+
buffer.set(groupId, []);
1060+
}
1061+
buffer.get(groupId).push(eventPackage);
1062+
1063+
// Set a timer to flush the buffer. If another event in the same group arrives, the timer will be reset.
1064+
constants.mutableRefs.eventFlushTimer = setTimeout(() => flushEventBuffer(groupId), 100); // 100ms debounce delay
10991065
}
11001066
}
11011067

@@ -1689,4 +1655,4 @@ export function setupCSS() {
16891655
*/
16901656
export function getActiveRemoteClientIDs() {
16911657
return new Set(cursorElements.keys());
1692-
}
1658+
}

‎src/addons/addons/collaboration/helpers/constants.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export const debugging = true;
1010
/**
1111
* The base URL for the WebSocket server used for Yjs collaboration.
1212
*/
13-
export const WEBSOCKETBASEURL = 'ws://localhost:4444/';
13+
export const WEBSOCKETBASEURL = 'wss://collaborator.codetorch.net/';
1414

1515
/**
1616
* Inactivity thresholds in milliseconds.
@@ -352,6 +352,10 @@ export const mutableRefs = {
352352
hasProcessedInitialProjectEvents: false, // Flag to track if initial project events (assets) have been processed.
353353
hasProcessedInitialBlockEvents: false, // Flag to track if initial Blockly events have been processed.
354354
alreadyRanSetup: false, // Flag to prevent re-running the main collaboration setup logic.
355+
356+
// --- Event Transaction Buffering ---
357+
eventTransactionBuffer: new Map(), // A map to buffer Blockly events by their group ID.
358+
eventFlushTimer: null, // A timer for debouncing the flushing of the event buffer.
355359
};
356360

357361
/**
@@ -401,4 +405,4 @@ export const TAB_USER_ICON_CLASS = 'collaboration-tab-user-icon';
401405
* A Map to store the container elements for user icons on each tab.
402406
* Key: tabIndex, Value: { container: HTMLElement, icons: Map<clientID, HTMLElement> }
403407
*/
404-
export const tabIconContainers = new Map();
408+
export const tabIconContainers = new Map();

‎src/addons/addons/collaboration/helpers/yEvents.js‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,26 @@ export function setupYEventsObserver() {
3838
// For custom events like 'shareBlocksToTarget', the VM is the primary dependency.
3939
if (!workspace && event.changes.added.some(item => item.content?.getContent()?.[0]?.event?.type)) {
4040
console.warn('Collab RX: Workspace not found, cannot apply remote standard Blockly events.');
41-
// Custom events that don't directly modify the workspace (like 'CUSTOM_REMOTE_SHARE_BLOCKS_CALL_TYPE')
42-
// can still proceed even if the workspace isn't fully ready.
4341
}
4442

4543
const allReceivedEventItemsInBatch = [];
4644
// Extract all individual event items from the Yjs transaction's added changes.
45+
// The content can now be a single item (an array which is our batch) or multiple items.
4746
event.changes.added.forEach(item => {
47+
// item.content.getContent() returns an array of items that were pushed in a transaction.
4848
if (item.content && typeof item.content.getContent === 'function') {
49-
item.content.getContent().forEach(eventItem => {
50-
allReceivedEventItemsInBatch.push(eventItem);
49+
const pushedItems = item.content.getContent();
50+
pushedItems.forEach(pushedItem => {
51+
// A "pushedItem" is now expected to be an array of event objects (a batch).
52+
if (Array.isArray(pushedItem)) {
53+
// It's a batch, so spread its contents into our processing list.
54+
allReceivedEventItemsInBatch.push(...pushedItem);
55+
} else {
56+
// For robustness, handle cases where a single, non-batched event might be sent.
57+
allReceivedEventItemsInBatch.push(pushedItem);
58+
}
5159
});
5260
} else {
53-
// Log a warning if an item in the Yjs change array has an unexpected structure.
5461
console.warn('Collab RX (yEvents observer): Received unexpected item structure in yEvents change:', item);
5562
}
5663
});
@@ -62,10 +69,6 @@ export function setupYEventsObserver() {
6269
let itemsToProcess = allReceivedEventItemsInBatch;
6370

6471
// --- Optimization logic for initial synchronization ---
65-
// This block attempts to optimize the initial load by only processing events
66-
// from the last 'savedProject' event onwards within the first batch of events.
67-
// This is crucial to avoid applying granular, potentially outdated, block changes
68-
// if a full project state was synced more recently.
6972
if (!constants.mutableRefs.hasProcessedInitialBlockEvents) {
7073
let lastSaveProjectIndexInBatch = -1;
7174
// Iterate backwards through the current batch to find the last 'savedProject' event.
@@ -83,17 +86,14 @@ export function setupYEventsObserver() {
8386
console.log(`Collab RX (yEvents observer): Initial event batch. Found 'savedProject'. Processing ${itemsToProcess.length} of ${allReceivedEventItemsInBatch.length} items from this batch, starting from the save event.`);
8487
}
8588
} else {
86-
// If no 'savedProject' marker is in this batch, process all events in the batch.
8789
if (constants.debugging) {
8890
console.log(`Collab RX (yEvents observer): Initial event batch. No 'savedProject' found in this specific batch. Processing all ${allReceivedEventItemsInBatch.length} items from this batch.`);
8991
}
9092
}
91-
// Mark that this initial processing logic has been applied for this observer.
9293
constants.mutableRefs.hasProcessedInitialBlockEvents = true;
9394
}
9495
// --- End of optimization logic ---
9596

96-
// Log the number of events being processed, indicating if filtering occurred.
9797
if (constants.debugging && itemsToProcess !== allReceivedEventItemsInBatch) {
9898
console.log(`Collab RX (yEvents observer): Filtered to process ${itemsToProcess.length} events.`);
9999
} else if (constants.debugging && constants.mutableRefs.hasProcessedInitialBlockEvents) {
@@ -725,4 +725,4 @@ export function processSpecificEvent(item) {
725725
// Log a warning for any event item that does not conform to expected structures.
726726
console.warn('Collab RX: Received malformed event item with no type and no event field:', item);
727727
}
728-
}
728+
}

0 commit comments

Comments
 (0)