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.

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

AtomicFileOutputStream does not overwrite file if exception occurred during write #9067

Merged
merged 8 commits into from
Aug 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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