Fixed move semantics of DownloadAttempt #2963
Conversation
JohanMabille
force-pushed
the
mirror_oci
branch
from
cccf902 to
52ebc4e
Compare
JohanMabille
force-pushed
the
mirror_oci
branch
from
52ebc4e to
83be8ea
Compare
JohanMabille
changed the title
| @@ -34,9 +34,11 @@ namespace mamba | |||
| using on_success_callback = std::function<bool(DownloadSuccess)>; | |||
| using on_failure_callback = std::function<bool(DownloadError)>; | |||
|
|
|||
| explicit DownloadAttempt(const DownloadRequest& request); | |||
| DownloadAttempt() = default; | |||
There was a problem hiding this comment.
I'm not totally sure but I think to link properly you need DownloadAttempt::DownloadAttempt()=default; in the cpp (moving the definition in the cpp).
If you get no link issues in msvc, ignore this comment.
| on_success_callback m_success_callback; | ||
| on_failure_callback m_failure_callback; | ||
| std::size_t m_retry_wait_seconds = std::size_t(0); | ||
| std::unique_ptr<CompressionStream> p_stream = nullptr; |
There was a problem hiding this comment.
BTW can this be a normal member now? or an optional?
There was a problem hiding this comment.
It cannot because CompressionStream is a polymorphic type.
| std::string m_last_modified; | ||
| }; | ||
|
|
||
| std::unique_ptr<Impl> p_impl = nullptr; |
There was a problem hiding this comment.
Note that it's already nullptr by default.
| DownloadError build_download_error(TransferData data) const; | ||
| DownloadSuccess build_download_success(TransferData data) const; | ||
|
|
||
| CURLHandle* p_handle = nullptr; |
There was a problem hiding this comment.
Isnt handle already a value/pointer? Do you need to point to it?
There was a problem hiding this comment.
With this change, the handle is now a member (value) of DownloadTracker, to avoid recreating it each time we build an attempt. But the attempt still needs it, thus this pointer.
| { | ||
| m_retry_wait_seconds = std::size_t(0); | ||
| return [impl = p_impl.get()](CURLMultiHandle& handle, CURLcode code) |
There was a problem hiding this comment.
If you find issues later with this lambda being called after impl have been destroyed, consider using task_synchronizer to guarantee that this does not ever happen.
No description provided.