Skip to content

Commit 4de8bff

Browse files
twentyTwotwentyTwo
authored
fix: implement write lock for concurrent entry modifications and improve branch change handling (#90)
1 parent de112b6 commit 4de8bff

2 files changed

Lines changed: 80 additions & 62 deletions

File tree

‎src/database.ts‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export interface SummaryData {
1919
export class Database {
2020
private context: vscode.ExtensionContext;
2121
private entries: TimeEntry[] | null = null;
22+
private writeLock: Promise<void> = Promise.resolve();
2223

2324
constructor(context: vscode.ExtensionContext) {
2425
this.context = context;
@@ -62,32 +63,37 @@ export class Database {
6263
return;
6364
}
6465

65-
const dateString = this.getLocalDateString(date);
66-
const entries = this.getEntries();
67-
68-
const existingEntryIndex = entries.findIndex(entry =>
69-
entry.date === dateString &&
70-
entry.project === project &&
71-
entry.branch === branch &&
72-
entry.language === language
73-
);
74-
75-
// Round to 2 decimal places to avoid floating point issues
76-
const roundedTime = Math.round(timeSpent * 100) / 100;
66+
// Use a write lock to prevent concurrent modifications
67+
this.writeLock = this.writeLock.then(async () => {
68+
const dateString = this.getLocalDateString(date);
69+
const entries = this.getEntries();
70+
71+
const existingEntryIndex = entries.findIndex(entry =>
72+
entry.date === dateString &&
73+
entry.project === project &&
74+
entry.branch === branch &&
75+
entry.language === language
76+
);
77+
78+
// Round to 2 decimal places to avoid floating point issues
79+
const roundedTime = Math.round(timeSpent * 100) / 100;
80+
81+
if (existingEntryIndex !== -1) {
82+
entries[existingEntryIndex].timeSpent += roundedTime;
83+
} else {
84+
entries.push({ date: dateString, project, timeSpent: roundedTime, branch, language });
85+
}
7786

78-
if (existingEntryIndex !== -1) {
79-
entries[existingEntryIndex].timeSpent += roundedTime;
80-
} else {
81-
entries.push({ date: dateString, project, timeSpent: roundedTime, branch, language });
82-
}
87+
try {
88+
await this.updateEntries(entries);
89+
console.log(`Saved ${roundedTime}min for ${project}/${branch}/${language} on ${dateString}`);
90+
} catch (error) {
91+
console.error('Error saving entry:', error);
92+
vscode.window.showErrorMessage('Failed to save time entry');
93+
}
94+
});
8395

84-
try {
85-
await this.updateEntries(entries);
86-
console.log(`Saved ${roundedTime}min for ${project}/${branch}/${language} on ${dateString}`);
87-
} catch (error) {
88-
console.error('Error saving entry:', error);
89-
vscode.window.showErrorMessage('Failed to save time entry');
90-
}
96+
await this.writeLock;
9197
}
9298

9399
getEntries(): TimeEntry[] {

‎src/timeTracker.ts‎

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class TimeTracker implements vscode.Disposable {
2929
private focusTimeoutSeconds: number = 180; // Default 3 minutes * 60 = 180 seconds
3030
private gitWatcher: GitWatcher | null = null;
3131
private branchCheckInterval: NodeJS.Timeout | null = null;
32-
private isCheckingBranch: boolean = false;
32+
private branchCheckPromise: Promise<void> | null = null;
3333
private lastUpdateTime: number = Date.now();
3434
private lastFocusTime: number = Date.now();
3535
private healthManager: HealthNotificationManager;
@@ -661,11 +661,14 @@ export class TimeTracker implements vscode.Disposable {
661661

662662
// Check for branch changes less frequently to reduce CPU load
663663
// Changed from 1 second to 5 seconds to reduce the number of git processes spawned
664+
// Only set interval if git setup was successful
664665
this.branchCheckInterval = setInterval(async () => {
665666
await this.checkBranchChanges();
666667
}, 5000);
667668

668669
} catch (error) {
670+
// Ensure interval is not running if git setup failed
671+
this.stopGitWatcher();
669672
this.logger.logEvent('branch_check_error', {
670673
project: this.currentProject,
671674
currentBranch: this.currentBranch,
@@ -681,50 +684,59 @@ export class TimeTracker implements vscode.Disposable {
681684

682685
private async checkBranchChanges() {
683686
// Prevent concurrent executions of branch checking
684-
if (this.isCheckingBranch || !this.gitWatcher || !this.isTracking) {
687+
if (this.branchCheckPromise || !this.gitWatcher || !this.isTracking) {
685688
return;
686689
}
687690

688-
try {
689-
this.isCheckingBranch = true;
690-
const branchInfo = await this.gitWatcher.git.branch();
691-
const currentBranch = branchInfo.current || 'unknown';
692-
693-
// If branch has changed
694-
if (currentBranch !== this.gitWatcher.lastKnownBranch) {
695-
// Log branch change event
696-
this.logger.logEvent('branch_changed', {
697-
project: this.currentProject,
698-
language: this.currentLanguage,
699-
oldBranch: this.gitWatcher.lastKnownBranch,
700-
newBranch: currentBranch
701-
});
691+
// Create a promise chain to serialize branch checks
692+
this.branchCheckPromise = (async () => {
693+
try {
694+
const branchInfo = await this.gitWatcher!.git.branch();
695+
const currentBranch = branchInfo.current || 'unknown';
702696

703-
// Save the current session with the old branch
704-
await this.saveCurrentSession(`branch change from ${this.gitWatcher.lastKnownBranch} to ${currentBranch}`);
697+
// If branch has changed
698+
if (currentBranch !== this.gitWatcher!.lastKnownBranch) {
699+
// Log branch change event
700+
this.logger.logEvent('branch_changed', {
701+
project: this.currentProject,
702+
language: this.currentLanguage,
703+
oldBranch: this.gitWatcher!.lastKnownBranch,
704+
newBranch: currentBranch
705+
});
705706

706-
// Update branch tracking
707-
this.gitWatcher.lastKnownBranch = currentBranch;
708-
this.currentBranch = currentBranch;
707+
// Save the current session with the old branch
708+
await this.saveCurrentSession(`branch change from ${this.gitWatcher!.lastKnownBranch} to ${currentBranch}`);
709709

710-
// Start a new session from this point
711-
this.startTime = Date.now();
710+
// Update branch tracking only after save completes
711+
this.gitWatcher!.lastKnownBranch = currentBranch;
712+
this.currentBranch = currentBranch;
713+
714+
// Start a new session from this point
715+
this.startTime = Date.now();
716+
}
717+
} catch (error) {
718+
this.logger.logEvent('branch_check_error', {
719+
project: this.currentProject,
720+
currentBranch: this.currentBranch,
721+
currentLanguage: this.currentLanguage,
722+
error: error instanceof Error ?
723+
`Branch check error: ${error.message}` :
724+
'Unknown branch check error',
725+
location: 'checkBranchChanges'
726+
});
727+
console.error('Error checking branch changes:', error);
728+
729+
// Stop git watcher on persistent errors to prevent resource waste
730+
if (error instanceof Error && error.message.includes('Not a git repository')) {
731+
this.stopGitWatcher();
732+
}
733+
} finally {
734+
// Always reset the promise to allow future checks
735+
this.branchCheckPromise = null;
712736
}
713-
} catch (error) {
714-
this.logger.logEvent('branch_check_error', {
715-
project: this.currentProject,
716-
currentBranch: this.currentBranch,
717-
currentLanguage: this.currentLanguage,
718-
error: error instanceof Error ?
719-
`Branch check error: ${error.message}` :
720-
'Unknown branch check error',
721-
location: 'checkBranchChanges'
722-
});
723-
console.error('Error checking branch changes:', error);
724-
} finally {
725-
// Always reset the flag to allow future checks
726-
this.isCheckingBranch = false;
727-
}
737+
})();
738+
739+
await this.branchCheckPromise;
728740
}
729741

730742
private stopGitWatcher() {

0 commit comments

Comments
 (0)