jijo-paulose / gwtupload

Automatically exported from code.google.com/p/gwtupload
Other
0 stars 0 forks source link

Race condition between progress and actual upload which sometimes results in getServerResponse returning null #89

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Intellij IDEA environment, debug, breakpoint inside: 
addOnFinishUploadHandler() 
    public void onFinish(IUploader uploader) {
      if (uploader.getStatus() == IUploadStatus.Status.SUCCESS) {
    }
2. File size is 66MB. 

Logging output:
INFO: onSubmitComplete: null

What version of the product are you using? On what operating system?
version 0.6.3 on Mac OS X

Race condition between progress and actual upload which sometimes results in 
getServerResponse returning null in 
addOnFinishUploadHandler() 
    public void onFinish(IUploader uploader) {
      if (uploader.getStatus() == IUploadStatus.Status.SUCCESS) {
    }

I can suggest the following patch that works for SVN trunk r851: 

--- a/GwtUpload/core/src/main/java/gwtupload/client/Uploader.java
+++ b/GwtUpload/core/src/main/java/gwtupload/client/Uploader.java
@@ -912,6 +912,14 @@ public class Uploader extends Composite implements 
IsUpdateable, IUploader, HasJ
       if (waitingForResponse) {
         return;
       }
+      if (successful) {
+        //log("Timer update: upload is succcessful")
+        if (serverResponse != null) {
+          //log("Timer update: finishing upload")
+          uploadFinished();
+        }
+        return;
+      }
       waitingForResponse = true;
       // Using a reusable builder makes IE fail because it caches the response
       // So it's better to change the request path sending an additional random parameter
@@ -1056,7 +1064,11 @@ public class Uploader extends Composite implements 
IsUpdateable, IUploader, HasJ
     } else if (Utils.getXmlNodeValue(doc, TAG_FINISHED) != null) {
       log("server response is: finished " + getFileName(), null);
       successful = true;
-      uploadFinished();
+      //log("progress reported finished uploading");
+      if (serverResponse != null) {
+        //log("response from server has been received");
+        uploadFinished();
+      }
       return;
     } else if (Utils.getXmlNodeValue(doc, TAG_PERCENT) != null) {
       lastData = now();

Original issue reported on code.google.com by ora...@gmail.com on 17 Jan 2011 at 2:00

Attachments:

GoogleCodeExporter commented 9 years ago
Is the fix working for you? Since it's a race condition, it's a little hard to 
verify any fix.....

We are experiencing the same problem and here's my thought so far,

we actually removed the listener to onFinish altogether and instead are now 
hooked up to the form submit completion event

                    } else if (uploader.getStatus() == Status.CHANGED) {
                        //add a submit complete handler as soon as a new uploader is added
                        u.getForm().addSubmitCompleteHandler( new SubmitCompleteHandler() {
                            @Override
                            public void onSubmitComplete( SubmitCompleteEvent event ) {
                                onUploadComplete( u );
                            }
                        });

This, however, still has the problem that the response returns empty sometimes. 
Seems to me that there's some race condition between the doGet and doPost on 
the server side.

Original comment by sandeep....@gmail.com on 21 Jan 2011 at 11:03

GoogleCodeExporter commented 9 years ago
I tried the code change you have suggested and still got the null response. In 
addition, the status row gets hidden out of sync so file upload entries on the 
screen were disappearing. In addition to the change I suggested, we overwrote 
some portions of Upoader.java and MultiUploader.java that seem to have fixed 
the issue. I will post the diffs here shortly.

Original comment by sandeep....@gmail.com on 31 Jan 2011 at 9:49

GoogleCodeExporter commented 9 years ago
I have noticed that my first patch makes the widget to disappear after upload 
has completed. I have created a different, more straight forward patch that 
seems works fine. See the attachment. 

Original comment by ora...@gmail.com on 1 Feb 2011 at 12:16

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by manuel.carrasco.m on 7 Feb 2011 at 6:30

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r862.

Original comment by manuel.carrasco.m on 13 Feb 2011 at 11:23

GoogleCodeExporter commented 9 years ago
Here's what we ended up doing to correct this. Question for the original 
reporter,

Were you using Multi or Single uploader?

We ended up making the attached changes to correct the issue (in addition to 
hooking up to submit event instead of finish event). The change in 
MultiUploader seems to be causing the problem. It's a well known issue with GWT 
that if you remove the form before the server response is received, it can 
cause problems. We have thoroughly tested this change in our environment under 
load and concurrency testing.

Original comment by sandeep....@gmail.com on 19 Feb 2011 at 4:13

Attachments:

GoogleCodeExporter commented 9 years ago
the other missing file

Original comment by sandeep....@gmail.com on 19 Feb 2011 at 4:15

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hello, it seems that your patches are related with revision r861.
This issue was fixed in r862 and r861. 

Please download last gwtupload snapshot, and check that the issue does not 
happen any more.

Thank you
- Manolo

Original comment by manuel.carrasco.m on 19 Feb 2011 at 7:50

GoogleCodeExporter commented 9 years ago
I am trying to ascertain that the fix will work for multiupliader too. If you 
or the original reporter can confirm, I'd be glad to test out the snapshot. 
Otherwise, I suggest reviewing the diff file for multiuploader.

Original comment by sandeep....@gmail.com on 19 Feb 2011 at 8:41

GoogleCodeExporter commented 9 years ago
With the fix in r862 and r861, uploader are never marked as finished until the 
post was completed.

Removing the form is safe, ant it avoids memory issues and other problems like 
submit twice the same file, or asking the user to resubmit the form when the he 
reloads the page.

Another matter if if we should remove the form or reuse it for a new upload, 
see issue#81.

Original comment by manuel.carrasco.m on 20 Feb 2011 at 9:42

GoogleCodeExporter commented 9 years ago
Originally the issue I experienced was with MultiUploader.

Original comment by ora...@gmail.com on 20 Feb 2011 at 12:53

GoogleCodeExporter commented 9 years ago
Originally the issue I experienced was with MultiUploader.

Original comment by ora...@gmail.com on 20 Feb 2011 at 12:53