search

GSConnect / gnome-shell-extension-gsconnect

KDE Connect implementation for GNOME
GNU General Public License v2.0
3.21k stars 259 forks source link

GSConnect cannot send multiple files at once #1025

Open demokritos opened 3 years ago

demokritos commented 3 years ago

Describe the bug

GSConnect cannot send multiple files at once. If I try sending multiple files to my phone, only one file is received by my phone. GSConnect shows "Transfer Successful" notification only for the first file, and "Transferring File" notifications for the other files remain.

Steps To Reproduce:

  1. Go to Nautilus
  2. Select two files
  3. Open context menu and click a mobile device below "Send to Mobile Device"
  4. Find received files on the mobile device

Expected behavior

Support Log

GSConnect: 44 (user)
GJS:       16601
Session:   x11
OS:        Arch Linux
--------------------------------------------------------------------------------
-- Journal begins at Mon 2020-06-29 18:42:08 KST, ends at Sun 2020-11-29 21:46:50 KST. --
11월 29 21:45:55 dbus-daemon[1186]: [system] Activating via systemd: service name='org.freedesktop.hostname1' unit='dbus-org.freedesktop.hostname1.service' requested by ':1.134' (uid=1000 pid=6348 comm="gjs /home/esrevinu/.local/share/gnome-shell/extens")
11월 29 21:45:55 kernel: audit: type=1334 audit(1606653955.260:214): prog-id=35 op=LOAD
11월 29 21:45:55 kernel: audit: type=1334 audit(1606653955.260:215): prog-id=36 op=LOAD
11월 29 21:45:55 audit: BPF prog-id=35 op=LOAD
11월 29 21:45:55 audit: BPF prog-id=36 op=LOAD
11월 29 21:45:55 systemd[1]: Starting Hostname Service...
11월 29 21:45:55 dbus-daemon[1186]: [system] Successfully activated service 'org.freedesktop.hostname1'
11월 29 21:45:55 audit[1]: SERVICE_START pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-hostnamed comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
11월 29 21:45:55 systemd[1]: Started Hostname Service.
11월 29 21:45:55 kernel: audit: type=1130 audit(1606653955.340:216): pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-hostnamed comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
11월 29 21:46:25 pcloud-drive-1.8.9-1.AppImage.desktop[2760]: Update for linux-x64-prod-v1.8.9 is not available
11월 29 21:46:25 systemd[1]: systemd-hostnamed.service: Succeeded.
11월 29 21:46:25 audit[1]: SERVICE_STOP pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-hostnamed comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
11월 29 21:46:25 kernel: audit: type=1131 audit(1606653985.660:217): pid=1 uid=0 auid=4294967295 ses=4294967295 msg='unit=systemd-hostnamed comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=success'
11월 29 21:46:25 audit: BPF prog-id=36 op=UNLOAD
11월 29 21:46:25 audit: BPF prog-id=35 op=UNLOAD
11월 29 21:46:25 kernel: audit: type=1334 audit(1606653985.813:218): prog-id=36 op=UNLOAD
11월 29 21:46:25 kernel: audit: type=1334 audit(1606653985.813:219): prog-id=35 op=UNLOAD
11월 29 21:46:39 gnome-shell[2338]: Ignoring excess values in shadow definition
11월 29 21:46:39 gnome-shell[2338]: Ignoring excess values in shadow definition
11월 29 21:46:50 gnome-shell[2338]: Window manager warning: WM_TRANSIENT_FOR window 0x4e007e3 for 0x4e0f3ee window override-redirect is an override-redirect window and this is not correct according to the standard, so we'll fallback to the first non-override-redirect window 0x4e00007.

System Details (please complete the following information):

GSConnect environment (if applicable):

Additional Notes:

Sharing multiple files between my two phones and from one of my phone to GSConnect works well.

andyholmes commented 3 years ago

Thanks for reporting. Looks like concurrent packets sent from Core.Channel.upload() are hitting G_IO_ERROR_PENDING, probably because the packet queue is on the device side.

demokritos commented 3 years ago

GSconnect seems to be sending multiple files one by one. On the other hand, if I send two files from a device's KDE connect, the receiving device notifies me once for the two files that it has received two files, rather than notifying for each file. Should GSconnect send multiple files in one session?

andyholmes commented 3 years ago

I'm not sure what you mean exactly, but that sounds like a different issue.

KyleBroflowski commented 2 years ago

@andyholmes , this still is not resolved. I am having issues on the latest versions. First file goes through, not the rest

andyholmes commented 2 years ago

Correct, you can consider all open issues unresolved :slightly_smiling_face:

I don't use GSConnect myself anymore and don't really contribute much code for that reason. A fix for this will probably have to come from the community.

ferdnyc commented 1 year ago

Mmm, I see now.

KDEConnect has a CompositeUploadFileJob.java class that, it seems, precedes multiple file transfers with a metadata packet that lists the number of files to expect, and the total size of those files. Then the multiple transfers can be handled as a group. That seems to be the part we're missing.

The multiple files are transferred as separate network packets, just as we're doing, but without that preceding kdeconnect.share.request.update packet containing the JSON metadata for the entire set of transfers, e.g.:

{
  "numberOfFiles": 2,
  "totalPayloadSize": 12345678
}

...each file gets treated separately, instead of as a group. And I don't see any sign of kdeconnect.share.request.update packet creation in GSConnect.

Edit: Oh, and each packet of the group is also supposed to have the numberOfFiles and totalPayloadSize for the group included in its body.

Edit2: I have some really ugly code for doing multi-file sends in my fork's multi-file-transfer branch. I tested it with my newly-upgraded-to-Fedora-38 laptop, and it seems to be working; Android reports "Received N files from " when the transfers complete.

But I'll probably put that aside until my development machine is similarly upgraded, so I can work on it more comfortably and get it cleaned up. IOW, don't expect an imminent PR, at least not quite yet.

andyholmes commented 1 year ago

KDEConnect has a CompositeUploadFileJob.java class...

Yeah, that's not really the root of the problem here, although it would help.

The real problem here is that I put the packet queue in GSConnectDevice (or whatever, its been awhile). The idea was device plugins could just fire-and-forget JSON packets and if the channel got swapped, then no packets were lost.

In practice, the device channel must write packets to itself to negotiate an auxiliary stream (i.e. file transfer), so it's hitting G_IO_ERROR_PENDING because there's nothing on the channel's side preventing concurrent writes (except GIO which throws the error).

If I were to do it again it would just be a traditional callback loop over an array. Trying to "add" performance and simplicity here backfired and ended up hiding what is probably the cause of 99% of the heisenbugs in GSConnect.

ferdnyc commented 1 year ago

@andyholmes Ah, I see. So the issue is contention even before getting to the Transfer stage that I was looking at.

I did notice that, on that auxiliary channel, the Transfer class supports both file uploads and downloads, potentially even intermingled within the same Transfer queue. Which... not only seems like unnecessary complexity, but makes the whole multiple-file transfer thing really squirrelly. I managed to kludge in multiple-file send, but even as I was doing it I was thinking that everything I was doing was going to make it even more difficult to cleanly support multiple-file receive as well.

To handle the multi-file thing cleanly, I was thinking it'd be best to split Transfer into separate Upload and Download subclasses, representing a single group of file transfers in exclusively one direction or the other. Then those classes could handle consuming or generating the multi-file metadata as well, since they know which end of the transfer they're on.

(Right now I have Share scanning all the files the user selected in the send-file dialog and computing the total payload size, because there at least I have the list of all the files that make up a group transfer, and I know that all of them are outgoing. Once those files get added to Transfer itself, that's no longer a safe assumption to make because every one of Transfer's _items can be an incoming or outgoing file — or even other type of non-file exchange, like a URL or plaintext message.)

If a Transfer instance actually tried to handle files coming in the other direction, or non-file exchanges, while in the middle of a group file send, I have a feeling things would probably break anyway. So it seems better to me if each shareFiles (formerly shareFile) action creates a new Upload (Transfer subclass) instance just to handle that group of (1-N) outgoing files, and each incoming file transfer request triggers creation of a Download (Transfer subclass) instance to accept and process the packets for those files. (And then each class can also handle multiple-file grouping in their particular direction.)

WDYT?

andyholmes commented 1 year ago

It's been awhile since I looked at the composite transfer code. I believe it goes something like:

  1. Uploading device may send kdeconnect.share.request.update with the (initial) transfer plan
  2. Either way, the device will send a kdeconnect.share.request packet with the payload and up-to-date totals at that time
  3. If files are added before the group completes, the device should send a new kdeconnect.share.request.update with updates. Additional kdeconnect.share.request packets may also contain updated numberOfFiles and totalPayloadSize fields

IIRC, there can never be more than one composite transfer at once (in a given direction). The transfer object will hang around and just keep having files appended to it, until all files complete.

Non-file transfer shouldn't interrupt file transfers, since they're distinguished by the filename/text/url fields. There's not an actual specification of course, but there are some corner cases like file uploads with open set to true, which "can't" be a part of a transfer group (not logically anyways, I think).

Having separate classes for upload/download groups makes sense, but I think the transfers themselves can be determined based on whether payloadTransferInformation is present, since the channel must set that field it will not be set on upload packets. This is how I did it in Valent, although GJS gives you a lot more freedom and you can make a new subclass in a ahlf-dozen lines :)

andyholmes commented 1 year ago

Also, keep in mind there will still need to be a "vanilla" transfer for notification icons, etc.

InklingGirl commented 1 year ago

This is happening to me on Ubuntu 22.04 LTS.

eedeidk commented 1 year ago

This issue is present on Ubuntu 23.04 also. First file goes others dont.

InklingGirl commented 1 year ago

This is happening to me on Ubuntu 22.04 LTS.

The most recent time I used it, it seems to have worked as intended just sending all files. Strange.

jakubklos77 commented 9 months ago

I am having the same issue and wanted to report this but found this issue instead. It works from the Android KDE connect app to GSConnect but not from GSConnect to Android. I am on Ubuntu 22.04. Any progress on this or should we simply put multiple files into one archive?

Slavomir9 commented 7 months ago

I have the same bug Bug report Hello, I use Ubuntu 23.10 I use Android 14 with installed KDE Connect. GNOME Shell 45.2 Bug report. GS Connect has issue send multiple files to android device from Nautilus file browser. Please download mkv video screen recording for understand https://mega.nz/file/JuJQGCZT#A_u172F6vvgiSRHyWbApZZkpaZ_jSzTtwB5CF1HSA6g

If I send single file, it work correct. If I was installed only KDE connect, via Doplhin file browser, sending of multiple files work correct. Actually I have installed only GS Connect (actually I have not installed KDE Conect parallelly) and sending multiple files not work correct. It send only part of files, not all files.

Repair it please.

Thank you very much

VorpalQ commented 5 months ago

This issue is still present on Ubuntu 24.04

Bug is nearly 4 years old now! ;)

jakubklos77 commented 5 months ago

Yes, no work has been done on this issue. I learned to use zip files instead...

ferdnyc commented 5 months ago

So I've dusted off my branch and started updating it to the current code base (which has evolved significantly since the code I have so far was written).

Still no promises regarding timeframe, but I'm hoping to get multiple file sending working from both the user menu and the file manager extension, plus directory transfers via temporary .zip file.

So, ideally if it all works the way I'm hoping, you could select three files and two directories in Nautilus, and the "Send to Device" action would do a 5-file transfer to the receiving device.

Still no progress reporting, so you wouldn't want to use it for big payloads.

Stay tuned. Maybe. 😉

jakubklos77 commented 5 months ago

That is great news, will gladly test when ready. Good luck

ferdnyc commented 5 months ago

directory transfers via temporary .zip file.

That would be a file-manager-only feature, BTW, as it'd be leveraging Python's zipfile module to create the temp archives. I don't feel like dealing with creating zip files from JavaScript.

ferdnyc commented 5 months ago

My multi-file-transfer branch now has a working implementation of multiple-file upload, including directory transfers in Nautilus via temporary .zip archive.

Somewhere along the way, however, I managed to break single-file transfers, so I don't recommend using it yet unless you're willing to always transfer at least 2 files/directories at a time from Nautilus, and not use the "Send file" option of the device menu at all. (At least, I assume the latter is broken as well. I haven't tested it.)

Other caveats:

darkdragon-001 commented 5 months ago

Did you consider using a streaming implementation for zip files? This should solve most of the problems. A quick googling lead me to https://pypi.org/project/stream-zip/.

ferdnyc commented 5 months ago

@darkdragon-001 Interesting thought, but ultimately it's not the Python code sending the data. That's done in the JavaScript code. And in multi-file transfers, it needs to send an update packet containing the item count and total file size for the transfer, I think it would be a tricky thing to pull off. Streaming makes more sense for a large single-file transfer, not as much when you're sending multiple files (without some container format).

Plus, like I said, it's just a terrible idea to be sending enormous amounts of data over the KDEConnect file-transfer link. Rather than attempting to eliminate any barriers to doing that, my inclination is to lean in to them instead.

Even the naming thing, I don't think is a really serious restriction. Why would anyone want to simultaneously transfer two folders both named "Pictures" to their phone? (And to be clear, this is only about conflicts in the SAME transfer operation, and it's only about the top-level names. The directories — and therefore the zip files — being transferred can contain files/directories with the same name, they just can't have the same name.)

I could generate random unique names for the zip files (in fact, my first version did just that), but that gets confusing because they'll have that name when they're stored on the other end of the link. It's not like the Android device will extract or rename them automatically. So the filenames matter, because they're visible to the user.

I'll probably write some code to do "Pictures (1).zip"-style renaming, in case of conflicts, even though I already feel kind of like it's a solution looking for a problem.

darkdragon-001 commented 5 months ago

For the naming: you could just compress the folder as well. So even when the archive has a random name, once it is decompressed, you will have the original folder name back.

ferdnyc commented 5 months ago

For the naming: you could just compress the folder as well. So even when the archive has a random name, once it is decompressed, you will have the original folder name back.

Already doing that, but I still don't think people will appreciate transferring folders only to end up with filenames like gsconnectaY56xZ78.zip. (And having to guess what they contain.)

Yes, I could name them all something that contained the folder name along with a random identifier, but that's no easier than just renaming only the (rare) conflicts to foldername (1).zip and so on.

ferdnyc commented 5 months ago

Already doing that

And apparently I should stop, because at least Samsung's file manager defaults to extracting .zip files into a directory named for the .zip. So if you send Documents.zip and Documents (1).zip both containing a Documents/ folder, you end up with this after extracting:

Documents (1)/
  Documents/
    <the files>
Documents/
  Documents/
    <the other files>

Anyway, I've pushed commits to handle conflict-renaming and fix single-file transfers, so the branch is now usable for testing.

ferdnyc commented 5 months ago

In fact, it's close enough now that I'll open a PR, so people can review and comment on the code there.

jakubklos77 commented 5 months ago

I have made all possible tests and LGTM. Thank you very much, it is totally usable

ferdnyc commented 5 months ago

Thanks @jakubklos77 !

I've been running with this build for a few days now, and while I don't do a LOT of file transfers, the ones I've performed as tests also seem fine. I just need to sort out what about this change breaks the automated tests, and fix it. Then the PR should be in good shape to merge.

RoyTheNewbie commented 4 months ago

I am not a coder, just a humble user of GSConnect. I had the same problem (i.e. of not being able to send more than single files). I thought it was JUST ME so having just discovered this thread I realise it's affecting all users. My question is, as there now seems to be an almost imminent fix to the problem what would I do to incorporate the fix? Would I have to reinstall GSConnect on my laptop? UPDATE: I think there is a setting for an automatic update to the Firefox extension: I set it. So I hope that is correct.

ferdnyc commented 4 months ago

@RoyTheNewbie

The change would only affect the file manager plugins (Nautilus and Nemo), updates to the Firefox extension wouldn't affect it.

Once the fix is incorporated (meaning, when PR #1821 first exits draft status, and then is merged into the repo), you'd have two choices: If you want to get the fix installed as soon as possible, you could manually install a build of GSConnect that includes that change. (There's some information in the project wiki about how to do that, and we can steer you in the right direction if you want to go that route.) Or, you can wait until the next release of GSConnect, which will then be published to extensions.gnome.org and it'll be auto-updated on your system... assuming you're currently running with it installed from there to your user account.

(By that I mean, some Linux distros package GSConnect as a system package, installed through their system package manager. For users of those packages, the update would have to come from the same source, and it'd also be installed using the system's packaging tool — apt, dnf, apk, etc.)

All of that also assumes you're running on a system with GNOME 46 and the latest GSConnect, which is currently v57. If you're using an older LTS distro with GSConnect v56 or lower, those versions are no longer supported so there won't be a compatible extension update. (GNOME 46 brought sweeping changes to the extension system, so the current extension code — which is where the fix would be made — can no longer run on any earlier GNOME releases.)

ferdnyc commented 4 months ago

(Oh, and if you are on an older GNOME release, just for fun those steps will probably be slightly different.)

RoyTheNewbie commented 4 months ago

@ferdnyc Well, first, thank you very much for responding in such detail. Thank you! Unfortunately, as an old chap who is not a techie I feel a bit overwhelmed by all the info. In my earlier comment I obviously have confused the Firefox browser extension with the GSConnect extension itself. I have had to remind myself of what I did to get where I am. If I recall it correctly I did install the GSConnect extension according to the guidance on https://www.howtogeek.com/keep-linux-pc-and-android-phone-synced-with-gsconnect/. So I DID install it myself from the extensions.gnome.org website. Thereafter I must also have installed the Firefox extension.

Whatever I did, things now are working very harmoniously. I power off both my laptop and phone each evening and have had to force myself to remember the sequence of actions I need to follow to reconnect my paired laptop and Android phone. I actually use the Extension Manager app on my laptop to reconnect the paired devices. I do not have the Extensions app that you refer to above. I just installed it but it did not behave like you described above i.e. "You can check your GSConnect version by running the Extensions app and clicking the "Extension details" button ([⋮]) next to GSConnect. It'll say "Version ##" in the popup that opens.". It did not look like that so maybe it was a different app. Can you tell me where to find the real Extensions app?

So, because I am a bit confused, I think I shall try to go the automatic update option. That is, sit back and see if the auto update happens. This a GREAT app and I appreciate all the effort that you have put in to fix the multiple files problem. THANK YOU!

UPDATE: Bad news for me. I just used my Extension Manager app to check on the version of GSConnect that I installed. It's version 50. I have also confirmed from the Settings/About option that my Gnome shell version is 42.9. So it seems that I am not going to get an update at all. :-( FYI, I sent a message to System76 who sold me the laptop, asking them how I can update the Gnome Shell version. I saw this https://release.gnome.org/46/. System76 confirmed that when I upgrade Ubuntu to 24.04 I shall be using Gnome 46 :-). Thanks again for all the info you provided.

ferdnyc commented 4 months ago

@RoyTheNewbie

Believe me, I sympathize. It's a complex series of moving parts that makes this stuff work (when it even does!), and the fact that things keep changing behind the scenes — at different speeds for everyone — only further muddies everything.

Case in point: Extensions was in sort of a nascent state in GNOME 42, and particularly in Ubuntu 20.04. It wasn't installed by default or included on the Live USB apparently... local extension-management was sort of mid-migration from GNOME Tweaks to the new, dedicated app when 20.04 released. By GNOME 44 or 45 the transition to Extensions was complete, Tweaks had dropped the overlapping functionality, and Extensions had become more robust than its initial versions.

Extension Manager is also a solid, third-party tool, which strongly inspired the official Extensions app, and predates it. It's far more capable than the proto-Extensions app on those earlier systems, as you've discovered.

24.04 will indeed bring GNOME 46, and compatibility with current GSConnect releases (and future, at least until we find out if GNOME 47 forces any new compatibility breakage).

RoyTheNewbie commented 4 months ago

@ferdnyc Thanks again for all the background info! I shall move up to 24.04 soon.