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

Close channel before trying to open again Exception is raised upon folder creation #1961

Open
Naoray opened this issue Nov 22, 2023 · 6 comments

Comments

@Naoray
Copy link

Naoray commented Nov 22, 2023

I am using the flysystem-sftp-v3 in a Laravel app which uploads files to a SFTP server around 500times a month. Occasionally I get the error Please close the channel (256) before trying to open it again from SSH2::openChannel() when trying to run Storage::disk(’sftp’)->makeDirectory(‘folder_name’).

I tried to check the code to understand what the error means and how to avoid it, but I wasn’t able to figure it out myself. Can someone explain to me or point me in the right direction on when this happens and why?

    RuntimeException Please close the channel (256) before trying to open it again 
    vendor/phpseclib/phpseclib/phpseclib/Net/SSH2.php:2885 phpseclib3\Net\SSH2::openChannel
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:550 phpseclib3\Net\SFTP::partial_init_sftp_connection
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:624 phpseclib3\Net\SFTP::init_sftp_connection
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:536 phpseclib3\Net\SFTP::precheck
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2931 phpseclib3\Net\SFTP::get_xstat_cache_prop
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2902 phpseclib3\Net\SFTP::get_stat_cache_prop
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2668 phpseclib3\Net\SFTP::is_dir
    vendor/league/flysystem-sftp-v3/SftpAdapter.php:112 League\Flysystem\PhpseclibV3\SftpAdapter::makeDirectory
    vendor/league/flysystem-sftp-v3/SftpAdapter.php:194 League\Flysystem\PhpseclibV3\SftpAdapter::createDirectory
    vendor/league/flysystem/src/Filesystem.php:95 League\Flysystem\Filesystem::createDirectory
    vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemAdapter.php:876 Illuminate\Filesystem\FilesystemAdapter::makeDirectory
    app/Jobs/UploadReleaseFolder.php:69 App\Jobs\UploadReleaseFolder::handle
@terrafrost
Copy link
Member

I can get a similar error if I try to do certain operations more than once. eg.

$sftp->openShell();
$sftp->openShell();

I mean, I should prob update it to work more like the DocBlock comments describe:

     * Creates an interactive shell
     *
     * Returns bool(true) if the shell was opened.
     * Returns bool(false) if the shell was already open.

Anyway, it almost feels like what's going is that the connection is getting interrupted and then phpseclib is trying to reconnect.

I think 8ecd156 should do the trick for you.

Thanks!

@Naoray
Copy link
Author

Naoray commented Nov 23, 2023

Thanks @terrafrost. Will upgrade and report back if the error happens again!

@Naoray
Copy link
Author

Naoray commented Dec 6, 2023

@terrafrost, the issue still persists :-/. Any other ideas?

@terrafrost
Copy link
Member

So I was able to test my theory and had to make one more tweak for it to work:

404f86f

That said, if my theory were true you'd also need to do $sftp->ping() as well. So like let's say you had this code:

$ssh = new SFTP('localhost', 22);
$ssh->login('user', 'pass');

echo $ssh->pwd() . "\n";
sleep(2);
//$ssh->ping();
echo $ssh->pwd();

And let's also say your sshd_config file had this in it:

ClientAliveInterval 1
ClientAliveCountMax 1

At that point the SSH server would disconnect between the two $ssh->pwd() commands and you'd need to reset it with $ssh->ping().

But is that what's actually going on in your case? idk. Outside of your stack trace I have very little information that I can work with.

In order to say more definitely I'm going to need to have the SSH logs. You can get them by doing define('NET_SSH2_LOGGING', 2) and then echo $ssh->getLog() after the error. Ideally I'd be able to see the whole SFTP exchange but if you're getting the error after you download or upload a 1GB file then I guess that's not realistic.

@sandcore-dev
Copy link

I just resolved a problem with a SFTP server using flysystem-sftp-v3 in Laravel. Since ping() is mentioned, I'll post it here.

The problem was that I got a Connection closed (by server) prematurely exception on one third-party server, but not another, after using allFiles() (although any command will probably throw this.) Using the phpseclib SFTP class directly worked fine for both servers.

After some debugging I found out that aforementioned server disconnected after executing ping(). This pinging happens in the SimpleConnectivityChecker class. Executing ping() resolved the problem because it will call reconnect(). This worked on both third-party servers.

So I came up with this workaround:

Storage::extend('sftp', function (Application $app, array $config) {
    $config['connectivityChecker'] = new class extends SimpleConnectivityChecker {
        public function isConnected(SFTP $connection): bool
        {
            if (parent::isConnected($connection)) {
                $connection->ping();
                return true;
            }

            return false;
        }
    };

    return Storage::createSftpDriver($config);
});

Hopefully this will help anyone.

@Naoray
Copy link
Author

Naoray commented Apr 5, 2024

@sandcore-dev I implemented your suggestion. I think I've also found an easier implementation, by just setting the filesystem.disks.{connectionName}.connectivityChecker config to new SimpleConnectivityChecker(usePing: true).

EDIT: Forget what I just wrote... you have to use extend, otherwise the config is not cacheable.

You could still modify your code to use

Storage::extend('sftp', function (Application $app, array $config) {
    $config['connectivityChecker'] = new SimpleConnectivityChecker(true);

    return Storage::createSftpDriver($config);
});

However, this didn't solve my issue. Here is my stack trace of the exception starting with the ->makeDirectory() call.

RuntimeException Please close the channel (256) before trying to open it again 
    vendor/phpseclib/phpseclib/phpseclib/Net/SSH2.php:2943 phpseclib3\Net\SSH2::open_channel
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:550 phpseclib3\Net\SFTP::partial_init_sftp_connection
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:624 phpseclib3\Net\SFTP::init_sftp_connection
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:536 phpseclib3\Net\SFTP::precheck
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2931 phpseclib3\Net\SFTP::get_xstat_cache_prop
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2902 phpseclib3\Net\SFTP::get_stat_cache_prop
    vendor/phpseclib/phpseclib/phpseclib/Net/SFTP.php:2668 phpseclib3\Net\SFTP::is_dir
    vendor/league/flysystem-sftp-v3/SftpAdapter.php:117 League\Flysystem\PhpseclibV3\SftpAdapter::makeDirectory
    vendor/league/flysystem-sftp-v3/SftpAdapter.php:199 League\Flysystem\PhpseclibV3\SftpAdapter::createDirectory
    vendor/league/flysystem/src/Filesystem.php:96 League\Flysystem\Filesystem::createDirectory
    vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemAdapter.php:871 Illuminate\Filesystem\FilesystemAdapter::makeDirectory

The additional ping() goes through and opens and closes the connection, but then in the SFTP@partial_init_sftp_connection() the open_channel() causes the exception.


Might be a dumb question, but can a SFTP connection handle two simultaneous write requests? It's already dawning on me that if I have two processes run in the queue trying to write simultaneously to the same sftp connection, this might cause the reported error. Can someone confirm this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants