Bug 362975

Summary: socket loses data if payload is too big
Product: [Frameworks and Libraries] frameworks-purpose Reporter: Harald Sitter <sitter>
Component: generalAssignee: Aleix Pol <aleixpol>
Status: VERIFIED FIXED    
Severity: major    
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: http://commits.kde.org/purpose/55f42c67641552f17c01677ea2254569e50c3a11 Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: patch using 1s waits
patchy with size announce before payload transfer

Description Harald Sitter 2016-05-12 09:40:42 UTC 
When making a 1920x1080 screenshot with spectacle and attempting to share it to imgur via purpose the posting never works. Smaller screenshots work.

Reason being that the lib will serialize the png into json and then send it over the socket, but that will more than likely exceed buffer limits of the socket, so writing happens not all at once but rather needs to be done sequentially as FIFO read. The pruproseprocess reading side however uses readAll() which only reads all bytes available for read, it does not wait for additional bytes!

The only way I found to make it work reliably is to have a readyReadWait and then readAll until the wait times out.

Reproducible: Always
Comment 1 Harald Sitter 2016-05-12 09:41:42 UTC 
Created attachment 98930 [details]
patch using 1s waits
Comment 2 Harald Sitter 2016-05-12 10:23:41 UTC 
Created attachment 98932 [details]
patchy with size announce before payload transfer

Best solution would be to have the lib first send one *line* with the amount of bytes it is going to write and then use a loop to read this amount of bytes with waitReady only acting as actual blocking method when we still have bytes to read, but they might not have been written yet.
Comment 3 Aleix Pol 2016-05-12 11:06:53 UTC 
Git commit 55f42c67641552f17c01677ea2254569e50c3a11 by Aleix Pol.
Committed on 12/05/2016 at 10:58.
Pushed by apol into branch '1.1'.

Improve the protocol we use to communicate between the config and the job

It wasn't doing so well when the payload exceeded a certain amount, now we
send first the size and wait to read for what is necessary.

Thanks to harald for contributing most of the patch!

M  +36   -0    autotests/alternativesmodeltest.cpp
M  +1    -0    autotests/alternativesmodeltest.h
M  +15   -8    src/externalprocess/processjob.cpp
M  +1    -0    src/externalprocess/processjob.h
M  +18   -3    src/externalprocess/purposeprocess_main.cpp

http://commits.kde.org/purpose/55f42c67641552f17c01677ea2254569e50c3a11