AtomicFileOutputStream does not overwrite file if exception occurred …

…during write (#9067)

Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com>

CHANGELOG.md

@@ -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
 

src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java

@@ -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);
             }
         });
 
@@ -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) {

src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java

@@ -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);
         }

src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java

@@ -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
@@ -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 {

src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java

@@ -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;
@@ -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();
     }
 
     /**
@@ -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);
         }
     }
 }

src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java

@@ -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<>();