xwikisas / application-filemanager

XWiki application to manage a hierarchy of folders and files
GNU Lesser General Public License v2.1
0 stars 4 forks source link

No warning message appears when a large file can't be deleted from File Manager UI #29

Open ane-gabriela opened 4 years ago

ane-gabriela commented 4 years ago

In regards to #7

Tested on XWiki 11.10.3 local instance and Cloud 11.3.6 and no error message appears when trying to delete a file of 50MB or larger.

After clicking the Delete button the UI refreshes after some time, the file remains - it's not removed and no error message appears.

NullError

polx commented 4 years ago

I tested 2.2.11 on multiple versions and could not reproduce it systematically. In some cases I saw out-of-memory-errors mentioned in the console. In one other case, I saw the error arrive as an object instead of as a string.

In the cases where I managed to reproduce it (only on xwiki 9 or 11), I could see that, as the screenshot shows, the error is not there in the status (the status' error is null).

In many other cases (macOS 10.14 with java 8 with xwiki 8.4, 9.11.9, 10.11.3, Ubuntu Linux with java 11 with xwiki 11.3, Debian Linux with java 8 and xwiki 11.3) I could upload and delete files as large as 600 Megabytes. I had errors at upload but none at delete.

From now I see the following possible choices:

1.- explore at the host of Gabi and debug there hopefully finding a pattern or a software difference 2.- enrich a strategy where the closing function call performs a check to see if the deletion was successful and if not display a message 3.- keep debugging all the possible cases by using the Docker testing (I started to look there then I realised that OOMs are not the really super candidate to try on docker)

My suggestion would be to just do number 2. @mflorea : What do you think?

mflorea commented 4 years ago

@polx I would start with 1. Talk with @ane-gabriela to get more details. Maybe there are some details we're missing or maybe she can give you access to her XWiki instance to test. Thanks.

polx commented 4 years ago

Hello @mflorea : I have now been able to investigate on @ane-gabriela's testing engine and can see the errors very well there.

Diagnostics: Memory management is, again, the problem: I always obtained OOM when duplicating the document's RCS. Do we really need the duplication before delete. It seems that this the cause.

Nonetheless, the error is only collected among the log and not within the error field. I am not sure why this is such and I believe that this may depend on the VM and the XWiki version. So I reiterate my suggestion number 2: enrich a strategy where the closing function call performs a check to see if the deletion was successful and if not display a message.

mflorea commented 4 years ago

Do we really need the duplication before delete. It seems that this the cause.

I doubt the clone() call is the issue. I bet on the deleteDocument() call. But anyway, it doesn't mater that much where is the source for OOM. The fact is: you can get an exception while deleting a document for many reasons. We need to be able to properly display an error message when that happens. Limiting the cases when the exception is thrown is another issue. So I think we need to investigate why $jobStatus.error is null. Maybe there is something to fix on the delete job.

Making an additional HTTP request at the end to check if the page still exists is not reliable: there is the edge case when a different user re-creates the page at the same time. In other words: the fact that the page exists some time after the delete finished doesn't imply (logically) that the delete has failed. It can simply mean that the page was re-created afterwards.

polx commented 4 years ago

Hello @mflorea , I'm afraid everything is happening outside of the filemanager application. Please see the file jobstatus-oom.xml.zip : the OutOfMemoryError is not thrown: It is just reported. This was produced on a patched FileManager where the runInternal of DeleteJob was enriched by a catch (Throwable t) { status.setError(t); throw t; } before the finally.

One with the released FileManager (2.2.11) is in oom-jobstatus-release.xml.zip. It is very similar.

In both cases an error is not thrown, the OOM happens in some array-copy down the road of deleteFile within XWikiAttachmentRCSArchive. This might well be outside of clone. You might be right there.

Thus changing the application can only be done with information provided to the user but not with catching an error.

the fact that the page exists some time after the delete finished doesn't imply (logically) that the delete has failed. It can simply mean that the page was re-created afterwards.

The implication is indeed wrong. But the wording could be carefully chosen so as not to confuse "the delete has failed" with "at the end of the delete operation, the file was still there".

Do you see an alternative?

mflorea commented 4 years ago

I'm afraid everything is happening outside of the filemanager application.

The OOM stacktrace starts from the File Manager application:

<trace>com.xpn.xwiki.XWiki.deleteDocument(XWiki.java:4415)</trace>                <trace>org.xwiki.filemanager.internal.DefaultFileSystem.delete(DefaultFileSystem.java:193)</trace>
<trace>org.xwiki.filemanager.internal.job.DeleteJob.deleteFile(DeleteJob.java:118)</trace>

so we know the OOM is thrown when a file is deleted. Sure, the exception may be caught somewhere, but the fact that you have it in the job log means you can determine if the job has failed or not (with high probability) by checking if there are log events of level ERROR. We can at least mention to the user that some errors were logged during the delete operation which could mean that the file was not deleted successfully. So instead of simply:

'error': $jobStatus.error,
'log': {},

we should also check the job logs. Either by passing them to the client (but it may be too expensive) or by setting the value of 'error' taking into account also the job logs.

ane-gabriela commented 3 years ago

Reproduced on XWiki 10.11.9 local instance - can't delete file and no warning is shown. I get a "java.lang.OutOfMemoryError: Java heap space" message in the XWiki console. Used a file of 44.6 MB

AfterDelete

NOTE: I've also tested on XWiki 12.10.4 with the same file and the documented is deleted, no errors appear.