Skip to content

Commit

Permalink
AtomicFileOutputStream does not overwrite file if exception occurred …
Browse files Browse the repository at this point in the history
…during write (#9067)

Co-authored-by: Christoph <[email protected]>
Co-authored-by: ThiloteE <[email protected]>
  • Loading branch information
3 people authored Aug 16, 2022
1 parent a3a886e commit 6a71395
Show file tree
Hide file tree
Showing 9 changed files with 193 additions and 82 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- The [HtmlToLaTeXFormatter](https://docs.jabref.org/finding-sorting-and-cleaning-entries/saveactions#html-to-latex) keeps single `<` characters.
- We fixed a performance regression when opening large libraries [#9041](https://github.com/JabRef/jabref/issues/9041)
- We fixed a bug where spaces are trimmed when highlighting differences in the Entries merge dialog. [koppor#371](https://github.com/koppor/jabref/issues/371)
- We fixed several bugs regarding the manual and the autosave of library files that sometimes lead to exceptions or data loss. [#8448](https://github.com/JabRef/jabref/issues/8484), [#8746](https://github.com/JabRef/jabref/issues/8746), [#6684](https://github.com/JabRef/jabref/issues/6684), [#6644](https://github.com/JabRef/jabref/issues/6644), [#6102](https://github.com/JabRef/jabref/issues/6102), [#6002](https://github.com/JabRef/jabref/issues/6000)
- We fixed an issue where applied save actions on saving the library file would lead to the dialog "The libary has been modified by another program" popping up [#4877](https://github.com/JabRef/jabref/issues/4877)

### Removed

Expand Down
24 changes: 13 additions & 11 deletions src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public DatabaseChangeMonitor(BibDatabaseContext database,
try {
fileMonitor.addListenerForFile(path, this);
} catch (IOException e) {
LOGGER.error("Error while trying to monitor " + path, e);
LOGGER.error("Error while trying to monitor {}", path, e);
}
});

Expand All @@ -75,16 +75,18 @@ public DatabaseChangeMonitor(BibDatabaseContext database,

@Override
public void fileUpdated() {
// File on disk has changed, thus look for notable changes and notify listeners in case there are such changes
ChangeScanner scanner = new ChangeScanner(database, dialogService, preferencesService, stateManager, themeManager);
BackgroundTask.wrap(scanner::scanForChanges)
.onSuccess(changes -> {
if (!changes.isEmpty()) {
listeners.forEach(listener -> listener.databaseChanged(changes));
}
})
.onFailure(e -> LOGGER.error("Error while watching for changes", e))
.executeWith(taskExecutor);
synchronized (database) {
// File on disk has changed, thus look for notable changes and notify listeners in case there are such changes
ChangeScanner scanner = new ChangeScanner(database, dialogService, preferencesService, stateManager, themeManager);
BackgroundTask.wrap(scanner::scanForChanges)
.onSuccess(changes -> {
if (!changes.isEmpty()) {
listeners.forEach(listener -> listener.databaseChanged(changes));
}
})
.onFailure(e -> LOGGER.error("Error while watching for changes", e))
.executeWith(taskExecutor);
}
}

public void addListener(DatabaseChangeListener listener) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@
public class AutosaveUiManager {
private static final Logger LOGGER = LoggerFactory.getLogger(AutosaveUiManager.class);

private final LibraryTab libraryTab;
private SaveDatabaseAction saveDatabaseAction;

public AutosaveUiManager(LibraryTab libraryTab) {
this.libraryTab = libraryTab;
this.saveDatabaseAction = new SaveDatabaseAction(libraryTab, Globals.prefs, Globals.entryTypesManager);
}

@Subscribe
public void listen(AutosaveEvent event) {
try {
new SaveDatabaseAction(libraryTab, Globals.prefs, Globals.entryTypesManager).save(SaveDatabaseAction.SaveDatabaseMode.SILENT);
this.saveDatabaseAction.save(SaveDatabaseAction.SaveDatabaseMode.SILENT);
} catch (Throwable e) {
LOGGER.error("Problem occurred while saving.", e);
}
Expand Down
49 changes: 29 additions & 20 deletions src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,21 @@ private boolean save(Path targetPath, SaveDatabaseMode mode) {
dialogService.notify(String.format("%s...", Localization.lang("Saving library")));
}

libraryTab.setSaving(true);
synchronized (libraryTab) {
if (libraryTab.isSaving()) {
// if another thread is saving, we do not need to save
return true;
}
libraryTab.setSaving(true);
}

try {
Charset encoding = libraryTab.getBibDatabaseContext()
.getMetaData()
.getEncoding()
.orElse(StandardCharsets.UTF_8);
// Make sure to remember which encoding we used.

// Make sure to remember which encoding we used
libraryTab.getBibDatabaseContext().getMetaData().setEncoding(encoding, ChangePropagation.DO_NOT_POST_EVENT);

// Save the database
Expand Down Expand Up @@ -227,28 +235,29 @@ private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding,
SavePreferences savePreferences = this.preferences.getSavePreferences()
.withSaveType(saveType);
BibDatabaseContext bibDatabaseContext = libraryTab.getBibDatabaseContext();
try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, encoding, savePreferences.shouldMakeBackup())) {
BibWriter bibWriter = new BibWriter(fileWriter, bibDatabaseContext.getDatabase().getNewLineSeparator());
BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(bibWriter, generalPreferences, savePreferences, entryTypesManager);

if (selectedOnly) {
databaseWriter.savePartOfDatabase(bibDatabaseContext, libraryTab.getSelectedEntries());
} else {
databaseWriter.saveDatabase(bibDatabaseContext);
}
synchronized (bibDatabaseContext) {
try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, encoding, savePreferences.shouldMakeBackup())) {
BibWriter bibWriter = new BibWriter(fileWriter, bibDatabaseContext.getDatabase().getNewLineSeparator());
BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(bibWriter, generalPreferences, savePreferences, entryTypesManager);

if (selectedOnly) {
databaseWriter.savePartOfDatabase(bibDatabaseContext, libraryTab.getSelectedEntries());
} else {
databaseWriter.saveDatabase(bibDatabaseContext);
}

libraryTab.registerUndoableChanges(databaseWriter.getSaveActionsFieldChanges());
libraryTab.registerUndoableChanges(databaseWriter.getSaveActionsFieldChanges());

if (fileWriter.hasEncodingProblems()) {
saveWithDifferentEncoding(file, selectedOnly, encoding, fileWriter.getEncodingProblems(), saveType);
if (fileWriter.hasEncodingProblems()) {
saveWithDifferentEncoding(file, selectedOnly, encoding, fileWriter.getEncodingProblems(), saveType);
}
} catch (UnsupportedCharsetException ex) {
throw new SaveException(Localization.lang("Character encoding '%0' is not supported.", encoding.displayName()), ex);
} catch (IOException ex) {
throw new SaveException("Problems saving: " + ex, ex);
}
} catch (UnsupportedCharsetException ex) {
throw new SaveException(Localization.lang("Character encoding '%0' is not supported.", encoding.displayName()), ex);
} catch (IOException ex) {
throw new SaveException("Problems saving: " + ex, ex);
return true;
}

return true;
}

private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set<Character> encodingProblems, SavePreferences.DatabaseSaveType saveType) throws SaveException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.jabref.logic.util.CoarseChangeFilter;
import org.jabref.logic.util.DelayTaskThrottler;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.AutosaveEvent;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
Expand All @@ -24,35 +24,47 @@ public class AutosaveManager {

private static final Logger LOGGER = LoggerFactory.getLogger(AutosaveManager.class);

private static final int DELAY_BETWEEN_AUTOSAVE_ATTEMPTS_IN_SECONDS = 31;

private static Set<AutosaveManager> runningInstances = new HashSet<>();

private final BibDatabaseContext bibDatabaseContext;

private final EventBus eventBus;
private final CoarseChangeFilter changeFilter;
private final DelayTaskThrottler throttler;
private final ScheduledThreadPoolExecutor executor;
private boolean needsSave = false;

private AutosaveManager(BibDatabaseContext bibDatabaseContext) {
this.bibDatabaseContext = bibDatabaseContext;
this.throttler = new DelayTaskThrottler(2000);
this.eventBus = new EventBus();
this.changeFilter = new CoarseChangeFilter(bibDatabaseContext);
changeFilter.registerListener(this);

this.executor = new ScheduledThreadPoolExecutor(2);
this.executor.scheduleAtFixedRate(
() -> {
if (needsSave) {
eventBus.post(new AutosaveEvent());
needsSave = false;
}
},
DELAY_BETWEEN_AUTOSAVE_ATTEMPTS_IN_SECONDS,
DELAY_BETWEEN_AUTOSAVE_ATTEMPTS_IN_SECONDS,
TimeUnit.SECONDS);
}

@Subscribe
public void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) {
if (!event.isFilteredOut()) {
throttler.schedule(() -> {
eventBus.post(new AutosaveEvent());
});
this.needsSave = true;
}
}

private void shutdown() {
changeFilter.unregisterListener(this);
changeFilter.shutdown();
throttler.shutdown();
executor.shutdown();
}

/**
Expand Down Expand Up @@ -88,7 +100,7 @@ public void unregisterListener(Object listener) {
eventBus.unregister(listener);
} catch (IllegalArgumentException e) {
// occurs if the event source has not been registered, should not prevent shutdown
LOGGER.debug("Proble, unregistering", e);
LOGGER.debug("Problem unregistering", e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class BackupManager {

private static final int MAXIMUM_BACKUP_FILE_COUNT = 10;

private static final int DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS = 20;
private static final int DELAY_BETWEEN_BACKUP_ATTEMPTS_IN_SECONDS = 19;

private static Set<BackupManager> runningInstances = new HashSet<>();

Expand Down
Loading

0 comments on commit 6a71395

Please sign in to comment.