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

Remove unsafe transmute of should_interrupt #1154

Closed
wants to merge 1 commit into from
Closed
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: 1 addition & 1 deletion gix-protocol/src/fetch/arguments/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ impl Arguments {
&mut self,
transport: &'a mut T,
add_done_argument: bool,
) -> Result<Box<dyn client::ExtendedBufRead + Unpin + 'a>, client::Error> {
) -> Result<Box<dyn client::ExtendedBufRead<'a> + Unpin + 'a>, client::Error> {
if self.haves.is_empty() {
assert!(add_done_argument, "If there are no haves, is_done must be true.");
}
Expand Down
2 changes: 1 addition & 1 deletion gix-protocol/src/fetch/arguments/blocking_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ impl Arguments {
&mut self,
transport: &'a mut T,
add_done_argument: bool,
) -> Result<Box<dyn client::ExtendedBufRead + Unpin + 'a>, client::Error> {
) -> Result<Box<dyn client::ExtendedBufRead<'a> + Unpin + 'a>, client::Error> {
if self.haves.is_empty() {
assert!(add_done_argument, "If there are no haves, is_done must be true.");
}
Expand Down
4 changes: 2 additions & 2 deletions gix-protocol/src/fetch/response/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::fetch::{

async fn parse_v2_section<T>(
line: &mut String,
reader: &mut (impl client::ExtendedBufRead + Unpin),
reader: &mut (impl client::ExtendedBufRead<'_> + Unpin),
res: &mut Vec<T>,
parse: impl Fn(&str) -> Result<T, response::Error>,
) -> Result<bool, response::Error> {
Expand Down Expand Up @@ -44,7 +44,7 @@ impl Response {
/// that `git` has to use to predict how many acks are supposed to be read. We also genuinely hope that this covers it all….
pub async fn from_line_reader(
version: Protocol,
reader: &mut (impl client::ExtendedBufRead + Unpin),
reader: &mut (impl client::ExtendedBufRead<'_> + Unpin),
client_expects_pack: bool,
wants_to_negotiate: bool,
) -> Result<Response, response::Error> {
Expand Down
8 changes: 4 additions & 4 deletions gix-protocol/src/fetch/response/blocking_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use crate::fetch::{
Response,
};

fn parse_v2_section<T>(
fn parse_v2_section<'a, T>(
line: &mut String,
reader: &mut impl client::ExtendedBufRead,
reader: &mut impl client::ExtendedBufRead<'a>,
res: &mut Vec<T>,
parse: impl Fn(&str) -> Result<T, response::Error>,
) -> Result<bool, response::Error> {
Expand Down Expand Up @@ -42,9 +42,9 @@ impl Response {
/// is to predict how to parse V1 output only, and neither `client_expects_pack` nor `wants_to_negotiate` are relevant for V2.
/// This ugliness is in place to avoid having to resort to an [an even more complex ugliness](https://github.com/git/git/blob/9e49351c3060e1fa6e0d2de64505b7becf157f28/fetch-pack.c#L583-L594)
/// that `git` has to use to predict how many acks are supposed to be read. We also genuinely hope that this covers it all….
pub fn from_line_reader(
pub fn from_line_reader<'a>(
version: Protocol,
reader: &mut impl client::ExtendedBufRead,
reader: &mut impl client::ExtendedBufRead<'a>,
client_expects_pack: bool,
wants_to_negotiate: bool,
) -> Result<Response, response::Error> {
Expand Down
8 changes: 5 additions & 3 deletions gix-protocol/src/fetch_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,10 @@ where
Ok(())
}

fn setup_remote_progress<P>(progress: &mut P, reader: &mut Box<dyn gix_transport::client::ExtendedBufRead + Unpin + '_>)
where
fn setup_remote_progress<P>(
progress: &mut P,
reader: &mut Box<dyn gix_transport::client::ExtendedBufRead<'_> + Unpin + '_>,
) where
P: NestedProgress,
P::SubProgress: 'static,
{
Expand All @@ -176,5 +178,5 @@ where
crate::RemoteProgress::translate_to_progress(is_err, data, &mut remote_progress);
gix_transport::packetline::read::ProgressAction::Continue
}
}) as gix_transport::client::HandleProgress));
}) as gix_transport::client::HandleProgress<'_>));
}
16 changes: 8 additions & 8 deletions gix-transport/src/client/async_io/bufread_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
/// A function `f(is_error, text)` receiving progress or error information.
/// As it is not a future itself, it must not block. If IO is performed within the function, be sure to spawn
/// it onto an executor.
pub type HandleProgress = Box<dyn FnMut(bool, &[u8]) -> ProgressAction>;
pub type HandleProgress<'a> = Box<dyn FnMut(bool, &[u8]) -> ProgressAction + 'a>;

/// This trait exists to get a version of a `gix_packetline::Provider` without type parameters,
/// but leave support for reading lines directly without forcing them through `String`.
Expand Down Expand Up @@ -44,11 +44,11 @@ pub trait ReadlineBufRead: AsyncBufRead {

/// Provide even more access to the underlying packet reader.
#[async_trait(?Send)]
pub trait ExtendedBufRead: ReadlineBufRead {
pub trait ExtendedBufRead<'a>: ReadlineBufRead {
/// Set the handler to which progress will be delivered.
///
/// Note that this is only possible if packet lines are sent in side band mode.
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>);
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>);
/// Peek the next data packet line. Maybe None if the next line is a packet we stop at, queryable using
/// [`stopped_at()`][ExtendedBufRead::stopped_at()].
async fn peek_data_line(&mut self) -> Option<io::Result<Result<&[u8], Error>>>;
Expand All @@ -70,8 +70,8 @@ impl<'a, T: ReadlineBufRead + ?Sized + 'a + Unpin> ReadlineBufRead for Box<T> {
}

#[async_trait(?Send)]
impl<'a, T: ExtendedBufRead + ?Sized + 'a + Unpin> ExtendedBufRead for Box<T> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>) {
impl<'a, T: ExtendedBufRead<'a> + ?Sized + 'a + Unpin> ExtendedBufRead<'a> for Box<T> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>) {
self.deref_mut().set_progress_handler(handle_progress)
}

Expand Down Expand Up @@ -101,7 +101,7 @@ impl<T: AsyncRead + Unpin> ReadlineBufRead
}

#[async_trait(?Send)]
impl<'a, T: AsyncRead + Unpin> ReadlineBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress> {
impl<'a, T: AsyncRead + Unpin> ReadlineBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress<'a>> {
async fn readline(&mut self) -> Option<io::Result<Result<PacketLineRef<'_>, gix_packetline::decode::Error>>> {
self.read_data_line().await
}
Expand All @@ -111,8 +111,8 @@ impl<'a, T: AsyncRead + Unpin> ReadlineBufRead for gix_packetline::read::WithSid
}

#[async_trait(?Send)]
impl<'a, T: AsyncRead + Unpin> ExtendedBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>) {
impl<'a, T: AsyncRead + Unpin> ExtendedBufRead<'a> for gix_packetline::read::WithSidebands<'a, T, HandleProgress<'a>> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>) {
self.set_progress_handler(handle_progress)
}
async fn peek_data_line(&mut self) -> Option<io::Result<Result<&[u8], Error>>> {
Expand Down
13 changes: 9 additions & 4 deletions gix-transport/src/client/async_io/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pin_project! {
on_into_read: MessageKind,
#[pin]
writer: gix_packetline::Writer<Box<dyn AsyncWrite + Unpin + 'a>>,
reader: Box<dyn ExtendedBufRead + Unpin + 'a>,
reader: Box<dyn ExtendedBufRead<'a> + Unpin + 'a>,
trace: bool,
}
}
Expand All @@ -43,7 +43,7 @@ impl<'a> RequestWriter<'a> {
/// If `trace` is true, `gix_trace` will be used on every written message or data.
pub fn new_from_bufread<W: AsyncWrite + Unpin + 'a>(
writer: W,
reader: Box<dyn ExtendedBufRead + Unpin + 'a>,
reader: Box<dyn ExtendedBufRead<'a> + Unpin + 'a>,
write_mode: WriteMode,
on_into_read: MessageKind,
trace: bool,
Expand Down Expand Up @@ -102,7 +102,7 @@ impl<'a> RequestWriter<'a> {
/// Discard the ability to write and turn this instance into the reader for obtaining the other side's response.
///
/// Doing so will also write the message type this instance was initialized with.
pub async fn into_read(mut self) -> std::io::Result<Box<dyn ExtendedBufRead + Unpin + 'a>> {
pub async fn into_read(mut self) -> std::io::Result<Box<dyn ExtendedBufRead<'a> + Unpin + 'a>> {
use futures_lite::AsyncWriteExt;
self.write_message(self.on_into_read).await?;
self.writer.inner_mut().flush().await?;
Expand All @@ -119,7 +119,12 @@ impl<'a> RequestWriter<'a> {
/// It's of utmost importance to drop the request writer before reading the response as these might be inter-dependent, depending on
/// the underlying transport mechanism. Failure to do so may result in a deadlock depending on how the write and read mechanism
/// is implemented.
pub fn into_parts(self) -> (Box<dyn AsyncWrite + Unpin + 'a>, Box<dyn ExtendedBufRead + Unpin + 'a>) {
pub fn into_parts(
self,
) -> (
Box<dyn AsyncWrite + Unpin + 'a>,
Box<dyn ExtendedBufRead<'a> + Unpin + 'a>,
) {
(self.writer.into_inner(), self.reader)
}
}
4 changes: 2 additions & 2 deletions gix-transport/src/client/async_io/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub trait TransportV2Ext {
capabilities: impl Iterator<Item = (&'a str, Option<impl AsRef<str>>)> + 'a,
arguments: Option<impl Iterator<Item = bstr::BString> + 'a>,
trace: bool,
) -> Result<Box<dyn ExtendedBufRead + Unpin + '_>, Error>;
) -> Result<Box<dyn ExtendedBufRead<'_> + Unpin + '_>, Error>;
}

#[async_trait(?Send)]
Expand All @@ -93,7 +93,7 @@ impl<T: Transport> TransportV2Ext for T {
capabilities: impl Iterator<Item = (&'a str, Option<impl AsRef<str>>)> + 'a,
arguments: Option<impl Iterator<Item = BString> + 'a>,
trace: bool,
) -> Result<Box<dyn ExtendedBufRead + Unpin + '_>, Error> {
) -> Result<Box<dyn ExtendedBufRead<'_> + Unpin + '_>, Error> {
let mut writer = self.request(WriteMode::OneLfTerminatedLinePerWriteCall, MessageKind::Flush, trace)?;
writer.write_all(format!("command={command}").as_bytes()).await?;
for (name, value) in capabilities {
Expand Down
16 changes: 8 additions & 8 deletions gix-transport/src/client/blocking_io/bufread_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
Protocol,
};
/// A function `f(is_error, text)` receiving progress or error information.
pub type HandleProgress = Box<dyn FnMut(bool, &[u8]) -> ProgressAction>;
pub type HandleProgress<'a> = Box<dyn FnMut(bool, &[u8]) -> ProgressAction + 'a>;

/// This trait exists to get a version of a `gix_packetline::Provider` without type parameters,
/// but leave support for reading lines directly without forcing them through `String`.
Expand All @@ -37,11 +37,11 @@ pub trait ReadlineBufRead: io::BufRead {
}

/// Provide even more access to the underlying packet reader.
pub trait ExtendedBufRead: ReadlineBufRead {
pub trait ExtendedBufRead<'a>: ReadlineBufRead {
/// Set the handler to which progress will be delivered.
///
/// Note that this is only possible if packet lines are sent in side band mode.
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>);
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>);
/// Peek the next data packet line. Maybe None if the next line is a packet we stop at, queryable using
/// [`stopped_at()`][ExtendedBufRead::stopped_at()].
fn peek_data_line(&mut self) -> Option<io::Result<Result<&[u8], Error>>>;
Expand All @@ -61,8 +61,8 @@ impl<'a, T: ReadlineBufRead + ?Sized + 'a> ReadlineBufRead for Box<T> {
}
}

impl<'a, T: ExtendedBufRead + ?Sized + 'a> ExtendedBufRead for Box<T> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>) {
impl<'a, T: ExtendedBufRead<'a> + ?Sized + 'a> ExtendedBufRead<'a> for Box<T> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>) {
self.deref_mut().set_progress_handler(handle_progress)
}

Expand All @@ -89,7 +89,7 @@ impl<T: io::Read> ReadlineBufRead for gix_packetline::read::WithSidebands<'_, T,
}
}

impl<'a, T: io::Read> ReadlineBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress> {
impl<'a, T: io::Read> ReadlineBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress<'a>> {
fn readline(&mut self) -> Option<io::Result<Result<PacketLineRef<'_>, gix_packetline::decode::Error>>> {
self.read_data_line()
}
Expand All @@ -99,8 +99,8 @@ impl<'a, T: io::Read> ReadlineBufRead for gix_packetline::read::WithSidebands<'a
}
}

impl<'a, T: io::Read> ExtendedBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>) {
impl<'a, T: io::Read> ExtendedBufRead<'a> for gix_packetline::read::WithSidebands<'a, T, HandleProgress<'a>> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>) {
self.set_progress_handler(handle_progress)
}
fn peek_data_line(&mut self) -> Option<io::Result<Result<&[u8], Error>>> {
Expand Down
4 changes: 2 additions & 2 deletions gix-transport/src/client/blocking_io/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,8 +516,8 @@ impl<H: Http, B: ReadlineBufRead + Unpin> ReadlineBufRead for HeadersThenBody<H,
}
}

impl<H: Http, B: ExtendedBufRead + Unpin> ExtendedBufRead for HeadersThenBody<H, B> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress>) {
impl<'a, H: Http, B: ExtendedBufRead<'a> + Unpin> ExtendedBufRead<'a> for HeadersThenBody<H, B> {
fn set_progress_handler(&mut self, handle_progress: Option<HandleProgress<'a>>) {
self.body.set_progress_handler(handle_progress)
}

Expand Down
8 changes: 4 additions & 4 deletions gix-transport/src/client/blocking_io/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::client::{ExtendedBufRead, MessageKind, WriteMode};
pub struct RequestWriter<'a> {
on_into_read: MessageKind,
writer: gix_packetline::Writer<Box<dyn io::Write + 'a>>,
reader: Box<dyn ExtendedBufRead + Unpin + 'a>,
reader: Box<dyn ExtendedBufRead<'a> + Unpin + 'a>,
trace: bool,
}

Expand All @@ -35,7 +35,7 @@ impl<'a> RequestWriter<'a> {
/// If `trace` is true, `gix_trace` will be used on every written message or data.
pub fn new_from_bufread<W: io::Write + 'a>(
writer: W,
reader: Box<dyn ExtendedBufRead + Unpin + 'a>,
reader: Box<dyn ExtendedBufRead<'a> + Unpin + 'a>,
write_mode: WriteMode,
on_into_read: MessageKind,
trace: bool,
Expand Down Expand Up @@ -89,7 +89,7 @@ impl<'a> RequestWriter<'a> {
/// Discard the ability to write and turn this instance into the reader for obtaining the other side's response.
///
/// Doing so will also write the message type this instance was initialized with.
pub fn into_read(mut self) -> std::io::Result<Box<dyn ExtendedBufRead + Unpin + 'a>> {
pub fn into_read(mut self) -> std::io::Result<Box<dyn ExtendedBufRead<'a> + Unpin + 'a>> {
self.write_message(self.on_into_read)?;
self.writer.inner_mut().flush()?;
Ok(self.reader)
Expand All @@ -105,7 +105,7 @@ impl<'a> RequestWriter<'a> {
/// It's of utmost importance to drop the request writer before reading the response as these might be inter-dependent, depending on
/// the underlying transport mechanism. Failure to do so may result in a deadlock depending on how the write and read mechanism
/// is implemented.
pub fn into_parts(self) -> (Box<dyn io::Write + 'a>, Box<dyn ExtendedBufRead + Unpin + 'a>) {
pub fn into_parts(self) -> (Box<dyn io::Write + 'a>, Box<dyn ExtendedBufRead<'a> + Unpin + 'a>) {
(self.writer.into_inner(), self.reader)
}
}
4 changes: 2 additions & 2 deletions gix-transport/src/client/blocking_io/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub trait TransportV2Ext {
capabilities: impl Iterator<Item = (&'a str, Option<impl AsRef<str>>)> + 'a,
arguments: Option<impl Iterator<Item = bstr::BString>>,
trace: bool,
) -> Result<Box<dyn ExtendedBufRead + Unpin + '_>, Error>;
) -> Result<Box<dyn ExtendedBufRead<'_> + Unpin + '_>, Error>;
}

impl<T: Transport> TransportV2Ext for T {
Expand All @@ -85,7 +85,7 @@ impl<T: Transport> TransportV2Ext for T {
capabilities: impl Iterator<Item = (&'a str, Option<impl AsRef<str>>)> + 'a,
arguments: Option<impl Iterator<Item = BString>>,
trace: bool,
) -> Result<Box<dyn ExtendedBufRead + Unpin + '_>, Error> {
) -> Result<Box<dyn ExtendedBufRead<'_> + Unpin + '_>, Error> {
let mut writer = self.request(WriteMode::OneLfTerminatedLinePerWriteCall, MessageKind::Flush, trace)?;
writer.write_all(format!("command={command}").as_bytes())?;
for (name, value) in capabilities {
Expand Down
18 changes: 4 additions & 14 deletions gix/src/remote/connection/fetch/receive_pack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,24 +410,14 @@ fn add_shallow_args(
Ok((shallow_commits, shallow_lock))
}

fn setup_remote_progress(
fn setup_remote_progress<'a>(
progress: &mut dyn crate::DynNestedProgress,
reader: &mut Box<dyn gix_protocol::transport::client::ExtendedBufRead + Unpin + '_>,
should_interrupt: &AtomicBool,
reader: &mut Box<dyn gix_protocol::transport::client::ExtendedBufRead<'a> + Unpin + 'a>,
should_interrupt: &'a AtomicBool,
) {
use gix_protocol::transport::client::ExtendedBufRead;
reader.set_progress_handler(Some(Box::new({
let mut remote_progress = progress.add_child_with_id("remote".to_string(), ProgressId::RemoteProgress.into());
// SAFETY: Ugh, so, with current Rust I can't declare lifetimes in the involved traits the way they need to
// be and I also can't use scoped threads to pump from local scopes to an Arc version that could be
// used here due to the this being called from sync AND async code (and the async version doesn't work
// with a surrounding `std::thread::scope()`.
// Thus there is only claiming this is 'static which we know works for *our* implementations of ExtendedBufRead
// and typical implementations, but of course it's possible for user code to come along and actually move this
// handler into a context where it can outlive the current function. Is this going to happen? Probably not unless
// somebody really wants to break it. So, with standard usage this value is never used past its actual lifetime.
#[allow(unsafe_code)]
let should_interrupt: &'static AtomicBool = unsafe { std::mem::transmute(should_interrupt) };
move |is_err: bool, data: &[u8]| {
gix_protocol::RemoteProgress::translate_to_progress(is_err, data, &mut remote_progress);
if should_interrupt.load(Ordering::Relaxed) {
Expand All @@ -436,5 +426,5 @@ fn setup_remote_progress(
ProgressAction::Continue
}
}
}) as gix_protocol::transport::client::HandleProgress));
}) as gix_protocol::transport::client::HandleProgress<'a>));
}