Skip to content

Fix hang when exporting at the CLI#10383

Merged
Siedlerchr merged 17 commits intomainfrom
fix-10380
Sep 16, 2023
Merged

Fix hang when exporting at the CLI#10383
Siedlerchr merged 17 commits intomainfrom
fix-10380

Conversation

@koppor
Copy link
Copy Markdown
Member

@koppor koppor commented Sep 14, 2023

Fixes #10380

There were two issues:

  • NPE when getting the main table preferences.
  • Hanging after a successful export.

The former was solved by using the right getter (e4ea759 (#10383)), the latter was solved by shutting down the file update monitor 18a3cd3 (#10383) (refs https://github.com/JabRef/jabref-issue-melting-pot/issues/290).

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 14, 2023
private final List<ParserResult> parserResults;

// Written once by processArguments()
private List<ParserResult> parserResults;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init with empty list?

@Siedlerchr
Copy link
Copy Markdown
Member

looks good so far, but would need testing with the binary.

@koppor
Copy link
Copy Markdown
Member Author

koppor commented Sep 15, 2023

looks good so far, but would need testing with the binary.

Test was done at #10380 (comment).

I wonder whether we could simply do System.exit(0); instead doing a proper shut down after proper processing. -- I currently don't see any disadvantages there, but maybe, I overlook something?

@koppor
Copy link
Copy Markdown
Member Author

koppor commented Sep 15, 2023

I tried it with System.exit(0); at 231b3a9 (#10383).

@koppor koppor mentioned this pull request Sep 15, 2023
2 tasks
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 15, 2023

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@Siedlerchr
Copy link
Copy Markdown
Member

the 60s shutdown is normally only the timeout if the tasks were not shutdown before:

public static void shutdownThreadPools() {
TASK_EXECUTOR.shutdown();
if (fileUpdateMonitor != null) {
fileUpdateMonitor.shutdown();
}
JabRefExecutorService.INSTANCE.shutdownEverything();
}
public static void stopBackgroundTasks() {
Telemetry.shutdown();
Unirest.shutDown();
}

@Siedlerchr Siedlerchr added this pull request to the merge queue Sep 16, 2023
Merged via the queue into main with commit 94bc5c2 Sep 16, 2023
@Siedlerchr Siedlerchr deleted the fix-10380 branch September 16, 2023 08:36
Siedlerchr added a commit to Jaovitosr/jabref that referenced this pull request Sep 18, 2023
* upstream/main:
  update javafx to 20.0.2
  [Bot] Update journal abbreviation lists (JabRef#10387)
  Fix hang when exporting at the CLI (JabRef#10383)
  Add tests for EntryComperatorTest (JabRef#10357)
  Update CSL styles
  Remove empty lines
  One more
  Refinements
  Update docs/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-89-run-with-intellij.md
  Add some more exports
  Refine IntelliJ howto
  Fix localization
  Adds LaTeX integrity check based on SnuggleTeX
  Fix casing "macOS" and "JabRef", vendor: JabRef e.V.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JabRef 5.10 breaks CLI export

2 participants