From 89b54a5430c3cea8a2ef2ea05583a94e79ca3f9a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 13:08:37 +0200 Subject: [PATCH 1/8] Add AtomficFileOutputStreamTest --- .../exporter/AtomicFileOutputStream.java | 48 +++++++++---- .../exporter/AtomicFileOutputStreamTest.java | 68 +++++++++++++++++++ 2 files changed, 101 insertions(+), 15 deletions(-) create mode 100644 src/test/java/org/jabref/logic/exporter/AtomicFileOutputStreamTest.java diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index 09b0e25f460..1477cb66ee2 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -3,6 +3,7 @@ import java.io.FileOutputStream; import java.io.FilterOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.channels.FileLock; import java.nio.channels.OverlappingFileLockException; import java.nio.file.Files; @@ -73,17 +74,34 @@ public class AtomicFileOutputStream extends FilterOutputStream { * @param keepBackup whether to keep the backup file after a successful write process */ public AtomicFileOutputStream(Path path, boolean keepBackup) throws IOException { - super(Files.newOutputStream(getPathOfTemporaryFile(path))); + this(path, getPathOfTemporaryFile(path), Files.newOutputStream(getPathOfTemporaryFile(path)), keepBackup); + } + /** + * Creates a new output stream to write to or replace the file at the specified path. The backup file is deleted when the write was successful. + * + * @param path the path of the file to write to or replace + */ + public AtomicFileOutputStream(Path path) throws IOException { + this(path, false); + } + + /** + * Required for proper testing + */ + AtomicFileOutputStream(Path path, Path pathOfTemporaryFile, OutputStream temporaryFileOutputStream, boolean keepBackup) throws IOException { + super(temporaryFileOutputStream); this.targetFile = path; - this.temporaryFile = getPathOfTemporaryFile(path); + this.temporaryFile = pathOfTemporaryFile; this.backupFile = getPathOfSaveBackupFile(path); this.keepBackup = keepBackup; try { // Lock files (so that at least not another JabRef instance writes at the same time to the same tmp file) if (out instanceof FileOutputStream) { - temporaryFileLock = ((FileOutputStream) out).getChannel().lock(); + // temporaryFileLock = ((FileOutputStream) out).getChannel().lock(); + // FIXME: Current implementation leads to a "AccessDeniedException" at "Files.move()" + temporaryFileLock = null; } else { temporaryFileLock = null; } @@ -92,15 +110,6 @@ public AtomicFileOutputStream(Path path, boolean keepBackup) throws IOException } } - /** - * Creates a new output stream to write to or replace the file at the specified path. The backup file is deleted when the write was successful. - * - * @param path the path of the file to write to or replace - */ - public AtomicFileOutputStream(Path path) throws IOException { - this(path, false); - } - private static Path getPathOfTemporaryFile(Path targetFile) { return FileUtil.addExtension(targetFile, TEMPORARY_EXTENSION); } @@ -183,7 +192,11 @@ public void close() throws IOException { PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_READ); if (Files.exists(targetFile)) { - Files.copy(targetFile, backupFile, StandardCopyOption.REPLACE_EXISTING); + try { + Files.copy(targetFile, backupFile, StandardCopyOption.REPLACE_EXISTING); + } catch (Exception e) { + LOGGER.warn("Could not create backup file {}", backupFile); + } if (FileUtil.IS_POSIX_COMPLIANT) { try { oldFilePermissions = Files.getPosixFilePermissions(targetFile); @@ -193,8 +206,13 @@ public void close() throws IOException { } } - // Move temporary file (replace original if it exists) - Files.move(temporaryFile, targetFile, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + try { + // Move temporary file (replace original if it exists) + Files.move(temporaryFile, targetFile, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (Exception e) { + LOGGER.warn("Could not move temporary file", e); + throw e; + } // Restore file permissions if (FileUtil.IS_POSIX_COMPLIANT) { diff --git a/src/test/java/org/jabref/logic/exporter/AtomicFileOutputStreamTest.java b/src/test/java/org/jabref/logic/exporter/AtomicFileOutputStreamTest.java new file mode 100644 index 00000000000..80086d77db9 --- /dev/null +++ b/src/test/java/org/jabref/logic/exporter/AtomicFileOutputStreamTest.java @@ -0,0 +1,68 @@ +package org.jabref.logic.exporter; + +import java.io.ByteArrayInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; + +import com.google.common.base.Strings; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; + +class AtomicFileOutputStreamTest { + + private static final String FIFTY_CHARS = Strings.repeat("1234567890", 5); + private static final String FIVE_THOUSAND_CHARS = Strings.repeat("A", 5_000); + + @Test + public void normalSaveWorks(@TempDir Path tempDir) throws Exception { + Path out = tempDir.resolve("normal-save.txt"); + Files.writeString(out, FIFTY_CHARS); + + try (AtomicFileOutputStream atomicFileOutputStream = new AtomicFileOutputStream(out)) { + InputStream inputStream = new ByteArrayInputStream(FIVE_THOUSAND_CHARS.getBytes()); + inputStream.transferTo(atomicFileOutputStream); + } + + // Written file still has the contents as before the error + assertEquals(FIVE_THOUSAND_CHARS, Files.readString(out)); + } + + @Test + public void originalContentExistsAtWriteError(@TempDir Path tempDir) throws Exception { + Path pathToTestFile = tempDir.resolve("error-during-save.txt"); + Files.writeString(pathToTestFile, FIFTY_CHARS); + + Path pathToTmpFile = tempDir.resolve("error-during-save.txt.tmp"); + + try (FileOutputStream outputStream = new FileOutputStream(pathToTmpFile.toFile())) { + FileOutputStream spiedOutputStream = spy(outputStream); + doAnswer(invocation -> { + // by writing one byte, we ensure that the `.tmp` file is created + outputStream.write(((byte[]) invocation.getRawArguments()[0])[0]); + outputStream.flush(); + throw new IOException(); + }).when(spiedOutputStream) + .write(Mockito.any(byte[].class), anyInt(), anyInt()); + + assertThrows(IOException.class, () -> { + try (AtomicFileOutputStream atomicFileOutputStream = new AtomicFileOutputStream(pathToTestFile, pathToTmpFile, spiedOutputStream, false); + InputStream inputStream = new ByteArrayInputStream(FIVE_THOUSAND_CHARS.getBytes())) { + inputStream.transferTo(atomicFileOutputStream); + } + }); + } + + // Written file still has the contents as before the error + assertEquals(FIFTY_CHARS, Files.readString(pathToTestFile)); + } +} From f6bd24d183682c52de5ca76d1d4348ed87252f02 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 21:16:45 +0200 Subject: [PATCH 2/8] Introduce check for "errorDuringWrite" to prevent overwriting the bib on issues during saving --- .../exporter/AtomicFileOutputStream.java | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java index 1477cb66ee2..5694d79b803 100644 --- a/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java +++ b/src/main/java/org/jabref/logic/exporter/AtomicFileOutputStream.java @@ -49,7 +49,7 @@ public class AtomicFileOutputStream extends FilterOutputStream { private static final Logger LOGGER = LoggerFactory.getLogger(AtomicFileOutputStream.class); private static final String TEMPORARY_EXTENSION = ".tmp"; - private static final String SAVE_EXTENSION = BackupFileType.SAVE.getExtensions().get(0); + private static final String SAVE_EXTENSION = "." + BackupFileType.SAVE.getExtensions().get(0); /** * The file we want to create/replace. @@ -60,13 +60,17 @@ public class AtomicFileOutputStream extends FilterOutputStream { * The file to which writes are redirected to. */ private final Path temporaryFile; + private final FileLock temporaryFileLock; /** * A backup of the target file (if it exists), created when the stream is closed */ private final Path backupFile; + private final boolean keepBackup; + private boolean errorDuringWrite = false; + /** * Creates a new output stream to write to or replace the file at the specified path. * @@ -74,11 +78,13 @@ public class AtomicFileOutputStream extends FilterOutputStream { * @param keepBackup whether to keep the backup file after a successful write process */ public AtomicFileOutputStream(Path path, boolean keepBackup) throws IOException { - this(path, getPathOfTemporaryFile(path), Files.newOutputStream(getPathOfTemporaryFile(path)), keepBackup); + // Files.newOutputStream(getPathOfTemporaryFile(path)) leads to a "sun.nio.ch.ChannelOutputStream", which does not offer "lock" + this(path, getPathOfTemporaryFile(path), new FileOutputStream(getPathOfTemporaryFile(path).toFile()), keepBackup); } /** - * Creates a new output stream to write to or replace the file at the specified path. The backup file is deleted when the write was successful. + * Creates a new output stream to write to or replace the file at the specified path. + * The backup file is deleted when write was successful. * * @param path the path of the file to write to or replace */ @@ -99,9 +105,7 @@ public AtomicFileOutputStream(Path path) throws IOException { try { // Lock files (so that at least not another JabRef instance writes at the same time to the same tmp file) if (out instanceof FileOutputStream) { - // temporaryFileLock = ((FileOutputStream) out).getChannel().lock(); - // FIXME: Current implementation leads to a "AccessDeniedException" at "Files.move()" - temporaryFileLock = null; + temporaryFileLock = ((FileOutputStream) out).getChannel().lock(); } else { temporaryFileLock = null; } @@ -126,7 +130,7 @@ public Path getBackup() { } /** - * Override for performance reasons. + * Overridden because of cleanup actions in case of an error */ @Override public void write(byte b[], int off, int len) throws IOException { @@ -134,6 +138,7 @@ public void write(byte b[], int off, int len) throws IOException { out.write(b, off, len); } catch (IOException exception) { cleanup(); + errorDuringWrite = true; throw exception; } } @@ -142,32 +147,36 @@ public void write(byte b[], int off, int len) throws IOException { * Closes the write process to the temporary file but does not commit to the target file. */ public void abort() { + errorDuringWrite = true; try { super.close(); Files.deleteIfExists(temporaryFile); Files.deleteIfExists(backupFile); } catch (IOException exception) { - LOGGER.debug("Unable to abort writing to file " + temporaryFile, exception); + LOGGER.debug("Unable to abort writing to file {}", temporaryFile, exception); } } private void cleanup() { - try { - Files.deleteIfExists(temporaryFile); - } catch (IOException exception) { - LOGGER.debug("Unable to delete file " + temporaryFile, exception); - } - try { if (temporaryFileLock != null) { temporaryFileLock.release(); } } catch (IOException exception) { - LOGGER.warn("Unable to release lock on file " + temporaryFile, exception); + // Currently, we always get the exception: + // Unable to release lock on file C:\Users\koppor\AppData\Local\Temp\junit11976839611279549873\error-during-save.txt.tmp: java.nio.channels.ClosedChannelException + LOGGER.debug("Unable to release lock on file {}", temporaryFile, exception); + } + try { + Files.deleteIfExists(temporaryFile); + } catch (IOException exception) { + LOGGER.debug("Unable to delete file {}", temporaryFile, exception); } } - // perform the final operations to move the temporary file to its final destination + /** + * perform the final operations to move the temporary file to its final destination + */ @Override public void close() throws IOException { try { @@ -184,6 +193,11 @@ public void close() throws IOException { } super.close(); + if (errorDuringWrite) { + // in case there was an error during write, we do not replace the original file + return; + } + // We successfully wrote everything to the temporary file, lets copy it to the correct place // First, make backup of original file and try to save file permissions to restore them later (by default: 664) Set oldFilePermissions = EnumSet.of(PosixFilePermission.OWNER_READ, From 1613ef88c954731d8ca5b1340f5663acc34d10e3 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 22:12:41 +0200 Subject: [PATCH 3/8] Remove senseless test --- .../model/database/event/AutosaveEventTest.java | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 src/test/java/org/jabref/model/database/event/AutosaveEventTest.java diff --git a/src/test/java/org/jabref/model/database/event/AutosaveEventTest.java b/src/test/java/org/jabref/model/database/event/AutosaveEventTest.java deleted file mode 100644 index 10ed6f709de..00000000000 --- a/src/test/java/org/jabref/model/database/event/AutosaveEventTest.java +++ /dev/null @@ -1,14 +0,0 @@ -package org.jabref.model.database.event; - -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertNotNull; - -public class AutosaveEventTest { - - @Test - public void givenNothingWhenCreatingThenNotNull() { - AutosaveEvent e = new AutosaveEvent(); - assertNotNull(e); - } -} From 003533493207fa97253b3e298143cf0394117bea Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 22:12:51 +0200 Subject: [PATCH 4/8] Fix typos --- .../org/jabref/logic/autosaveandbackup/AutosaveManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java index 3e53d41357d..3633cfb7472 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java +++ b/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java @@ -34,7 +34,7 @@ public class AutosaveManager { private AutosaveManager(BibDatabaseContext bibDatabaseContext) { this.bibDatabaseContext = bibDatabaseContext; - this.throttler = new DelayTaskThrottler(2000); + this.throttler = new DelayTaskThrottler(2_000); this.eventBus = new EventBus(); this.changeFilter = new CoarseChangeFilter(bibDatabaseContext); changeFilter.registerListener(this); @@ -88,7 +88,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); } } } From 7f375ae0385750ea090394c1a4742d8eb4bfc259 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 22:40:08 +0200 Subject: [PATCH 5/8] Fix Autosave in parallel to saving through synchronized Co-authored-by: Christoph Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com> --- .../jabref/gui/dialogs/AutosaveUiManager.java | 6 ++--- .../gui/exporter/SaveDatabaseAction.java | 12 +++++++-- .../autosaveandbackup/AutosaveManager.java | 26 ++++++++++++++----- .../autosaveandbackup/BackupManager.java | 2 +- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java b/src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java index 7aee99be695..a7bc85890ee 100644 --- a/src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java +++ b/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); } diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index e22f5790ceb..d8c9bce37ba 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/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 diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java index 3633cfb7472..990da5c2db4 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java +++ b/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 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(2_000); 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(); } /** diff --git a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java index 750f201ee83..5b3f0cd8863 100644 --- a/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java +++ b/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 runningInstances = new HashSet<>(); From 98b9c2ec52590600ec1fea263f0c870d0a86470e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 23:03:25 +0200 Subject: [PATCH 6/8] Have DatabaseChangeMonitor wait for complete write and complete application of save actions Co-authored-by: Christoph Co-authored-by: ThiloteE <73715071+ThiloteE@users.noreply.github.com> --- .../gui/collab/DatabaseChangeMonitor.java | 24 ++++++------ .../gui/exporter/SaveDatabaseAction.java | 37 ++++++++++--------- 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java index 575307bcda8..5cff210b5d0 100644 --- a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java +++ b/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) { diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index d8c9bce37ba..84bf9346e23 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -235,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 encodingProblems, SavePreferences.DatabaseSaveType saveType) throws SaveException { From 0a3e8459ab489b82c3a1f2a66e515fc9986baf7f Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Tue, 16 Aug 2022 23:32:09 +0200 Subject: [PATCH 7/8] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 560c53118de..7d20ad056bb 100644 --- a/CHANGELOG.md +++ b/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 popping up "The libary has been modified by another program" [#4877](https://github.com/JabRef/jabref/issues/4877) ### Removed From 5b2b0a997852fb481ddf1c1f734c1913778a9184 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Aug 2022 23:33:45 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d20ad056bb..a869d7a09f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,8 +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 popping up "The libary has been modified by another program" [#4877](https://github.com/JabRef/jabref/issues/4877) +- 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