SFML community forums

General => Feature requests => Topic started by: Edgaru089 on August 08, 2017, 03:17:49 am

Title: Asynchronously transfer files via ftp
Post by: Edgaru089 on August 08, 2017, 03:17:49 am
Is it possible for sf::Ftp to transfer(download/upload) files in a separate thread?
Something like this:
sf::Ftp ftp;
ftp.connect("ftp.server.com");
ftp.login("username","password");

ftp.startDownload("remotefile.txt", "local/path");

while(ftp.isTransfering())
    std::cout<<"\rStatus: "<<ftp.getTransferPercentage()<<"%";
std::cout<<std::endl<<"Done."<<std::endl;
 
Title: Re: Asynchronously transfer files via ftp
Post by: Laurent on August 08, 2017, 07:57:23 am
Asynchronous FTP transfers has been planned for ages... but since it's not a critical component of SFML, nobody ever started working on it. Have you tried to perform the download in a thread? You won't get any feedback (progress), but at least it won't block your main thread.
Title: Re: Asynchronously transfer files via ftp
Post by: Edgaru089 on August 08, 2017, 09:23:00 am
I forked the SFML git repository a few months ago (https://github.com/Edgaru089/SFML (https://github.com/Edgaru089/SFML)), and I'm currently working on it. Should I start a issue first? Or just simply start a pull request when I'm ready?
BTW, should I use std::thread or sf::Thread to perform a transfer?
Title: Re: Asynchronously transfer files via ftp
Post by: Laurent on August 08, 2017, 10:04:49 am
Quote
I forked the SFML git repository a few months ago, and I'm currently working on it.
That's great :)

Quote
Should I start a issue first? Or just simply start a pull request when I'm ready?
We like to discuss things before they get implemented, so that we can agree on the public API before writing any code. You can also submit a finalized implementation if you prefer, but then there's the possibility that you end up rewriting parts of it after our review ;)

For example, I wouldn't implement it the way you proposed in your first post, as it cannot handle multiple transfers at the same time; why implement parallel transfers if at the end you can only perform one at a time?

Quote
BTW, should I use std::thread or sf::Thread to perform a transfer?
Since SFML 2 is still stuck at C++03, use sf::Thread please.
Title: Re: Asynchronously transfer files via ftp
Post by: Edgaru089 on August 08, 2017, 10:39:36 am
Quote
Since SFML 2 is still stuck at C++03, use sf::Thread please.
Ouch. I think that's one of the major reasons why nobody ever started working on it.

I just made&pushed a implementation using std::thread:
sf::Ftp::TransferState state;
server.downloadAsynced(filename, directory,sf::Ftp::Binary,state);

while (!state.isTransferCompleted())
    std::printf("\r%.2f KB / Total %.2f KB", (float)state.getTransferedSize() / 1024.0f, (float)fileSize / 1024.0f);

std::printf("\nCompleted", (float)fileSize / 1024.0f, (float)fileSize / 1024.0f);
 
Title: Re: Asynchronously transfer files via ftp
Post by: korczurekk on August 08, 2017, 12:59:15 pm
Ftp::TransferState::m_pointerToTransferedSize seems suspicious to me.
Is your code thread safe? I see a few possible data races in Ftp::downloadAsynced, Ftp::download, Ftp::TransferState::getTransferedSize and Ftp::TransferState::isTransferCompleted. All of them use said m_pointerToTransferedSize without any synchronization.
While it will probably work, it looks like an undefined behavior to me. No idea how I'd fix it tho, mutex is an overkill and neither SFML nor C++03 provide atomics.
Title: Re: Asynchronously transfer files via ftp
Post by: Edgaru089 on August 08, 2017, 03:11:54 pm
I think this is thread safe, for only one thread would write data at once, and there's only one primitive data object (thus one memory block). The other threads are only reading the data, so there should be no problem with thread safety.
Title: Re: Asynchronously transfer files via ftp
Post by: korczurekk on August 09, 2017, 04:26:39 pm
Well, I guess it's not a problem with types so small, that reading in the exact moment of writing is virtually impossible.
Title: Re: Asynchronously transfer files via ftp
Post by: Passer By on October 26, 2017, 01:32:46 pm
Sorry for necroing something a billion years old.

I think this is thread safe, for only one thread would write data at once, and there's only one primitive data object (thus one memory block). The other threads are only reading the data, so there should be no problem with thread safety.

Writing and reading at the same time is a data race, and it's UB. There is no way around having atomics for flags, and that probably means relying on non-standard sources for them before C++11.

Looking at https://github.com/Edgaru089/SFML/blob/master/src/SFML/Network/Ftp.cpp (https://github.com/Edgaru089/SFML/blob/master/src/SFML/Network/Ftp.cpp).

void Ftp::downloadAsynced(const std::string & remoteFile, const std::string& localPath, TransferMode mode, TransferState& state)
{
    std::thread downloadThread(&Ftp::download, this, remoteFile, localPath, mode, std::ref(state));
    downloadThread.detach();

    // Wait until the transfer has started
    while(state.isTransferCompleted())
        sleep(milliseconds(5));
}
 

Can be legally optimized by the compiler into

void Ftp::downloadAsynced(const std::string & remoteFile, const std::string& localPath, TransferMode mode, TransferState& state)
{
    std::thread downloadThread(&Ftp::download, this, remoteFile, localPath, mode, std::ref(state));
    downloadThread.detach();

    // isTransferCompleted has no optimization barriers in it
    if(state.isTransferCompleted())
        while(true)
            sleep(milliseconds(5));
}
 

This isn't all data races can do to wreak your program, don't do it.

PS: gcc 7.2 does indeed make that optimization under O2 (https://godbolt.org/g/33fuDe)