Skip to content

Commit

Permalink
Remove questionable unsafe impl Send
Browse files Browse the repository at this point in the history
`HistoryCompleter` is not `Send` due to it containing a reference to a
`&dyn History` which was only `trait History: Send`.
Here the property of a type T needing to implement `Sync` for its `&T`
to be `Send` comes into play. Thus this `unsafe impl Send` is unsound.

The change of making `trait History: Send + Sync` makes
`HistoryCompleter: Send` automatically and should be compatible for all
sound implementations.
  • Loading branch information
sholderbach committed Jan 2, 2025
1 parent 1800dcd commit a2a31ab
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 5 deletions.
4 changes: 0 additions & 4 deletions src/completion/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ const SELECTION_CHAR: char = '!';
// It pulls data from the object that contains access to the History
pub(crate) struct HistoryCompleter<'menu>(&'menu dyn History);

// Safe to implement Send since the HistoryCompleter should only be used when
// updating the menu and that must happen in the same thread
unsafe impl Send for HistoryCompleter<'_> {}

fn search_unique(
completer: &HistoryCompleter,
line: &str,
Expand Down
2 changes: 1 addition & 1 deletion src/history/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl SearchQuery {

/// Represents a history file or database
/// Data could be stored e.g. in a plain text file, in a `JSONL` file, in a `SQLite` database
pub trait History: Send {
pub trait History: Send + Sync {
/// save a history item to the database
/// if given id is None, a new id is created and set in the return value
/// if given id is Some, the existing entry is updated
Expand Down

0 comments on commit a2a31ab

Please sign in to comment.