View Issue Details

IDProjectCategoryView StatusLast Update
0019104MediaMonkey 5Extensions frameworkpublic2022-05-25 20:52
Reporterzvezdan Assigned To 
PriorityhighSeverityminorReproducibilityN/A
Status resolvedResolutionreopened 
Fixed in Version5.0.4 
Summary0019104: app.filesystem.fileExists method doesn't exists and it is needed (synchronous)
Descriptionapp.filesystem.fileExists method is mentioned in web API, but it doesn't exist. There is the fileExistsAsync method, but I need a synchronous version.

There is already dirExists method that is working synchronously, so I don't see any reason why we cannot have the same for testing files.
TagsNo tags attached.
Fixed in build2650

Relationships

related to 0019102 resolvedLudek renameFile method freezes the program when trying to overwrite a file 

Activities

Ludek

2022-05-24 19:24

developer   ~0068218

Last edited: 2022-05-24 19:26

View 3 revisions

Thanks, corrected the documentation, it should be fileExistsAsync
Marked dirExists as LEGACY and added dirExistsAsync

=> Fixed in 5.0.4.2650

As for the need to async methods: For files on attached network drive (NAS) it can take up 20 seconds before the drive spins (wake up) and the fileExists returns a value.
Therefore it needs to be async to not freeze the UI meanwhile.
You can easily await the async functions: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function

zvezdan

2022-05-25 13:43

updater   ~0068234

I think you should have them both at disposal and not to worry how they will be used. It would be scripter's responsibility if his code freezes UI or not.

If I have code which is executing in a modal dialog box, the rest of the program is unavailable anyway. If I have heavy database queries, the program will be unavailable until they are finished anyway. If I have very sensitive database manipulation in the script, I would disable all commands in the dialog box to disallow any interaction of user with the program. Which is a reason why I suggest users of some of my add-ons to stop any activity with the program while my add-on is executing, including playback and auto-scan of files.

I already have implement solution with async/await, but it is such PITA for my long code which has direct dependency on the result of such lines. It is especially problematic because await doesn't give any guaranty that the next line will be executed immediately after it. The next line will be executed after it for sure, but not always "immediately" after it because the program could execute some other code between them asynchronously which could alter logic of the code drastically.

Ludek

2022-05-25 17:56

developer   ~0068250

Last edited: 2022-05-25 17:58

View 3 revisions

I am not keen adding/using the synchronous variants especially when whole MM5 app was designed in UI non-blocking way.
It currently don't access DB / filesystem from the UI thread at all, but it is always asynchronous background task doing the job.
In addition freezing the UI would generate freeze log to submit anyway.

As for the sensitive DB operation, seeing that currently there are methods like beginTransaction/commitTransaction : https://www.mediamonkey.com/docs/api/classes/DB.html
although haven't tested I guess this won't work currently as it needs to be called on the same thread/task, so we should rather abandon beginTransaction/commitTransaction and introduce
app.db.executeTransactionAsync (in addition to existing app.db.executeQueryAsync)
that would perform the SQL in a transaction with possibility to prevent other threads/tasks to access the DB and wait.

zvezdan

2022-05-25 18:08

updater   ~0068251

I am working on the add-on that is using beginTransaction/commitTransaction right now, although I didn't test that part yet.

When are you expecting to implement their Async versions?

I suppose app.db.executeQueryAsync also prevents other threads/tasks to access the DB and wait, right?

By the way, as I said, you don't need to worry how such synchronous commands could perform in scripts. If you don't like them, you don't need to use them in your code. But they sould be available for scripters if they want them. You are looking at your program as a web browser all the time. Well, it is not a web browser, it is a media organizer and some things are better when executed synchronously.

Ludek

2022-05-25 18:29

developer   ~0068255

Re executeQueryAsync: Yes, it acquires write lock and executes the query in write lock exclusivelly.

As for the beginTransaction/commitTransaction, I see that it is part of our QUnit tests and the tests passes so it should be OK to use it like this:
image.png (125,002 bytes)   
image.png (125,002 bytes)   

zvezdan

2022-05-25 18:37

updater   ~0068257

Good to know. I already have something like that, but as I said I didn't test it yet.

By the way, I just saw that you have insertQueryAsync. Was that really necessary? Why What would happen if I use executeQueryAsync with INSERT query?

The old program had ExecSQL for all executable queries including INSERT.

Ludek

2022-05-25 18:41

developer   ~0068258

insertQuery is same as executeQuery with difference that it returns ID of the newly inserted row:
image-2.png (10,706 bytes)   
image-2.png (10,706 bytes)   

Ludek

2022-05-25 18:41

developer   ~0068259

And as for the original request of introducing synchronous version of fileExists: I am not keen adding it because of my arguments above (can take 20 seconds for files on NAS, can generate freeze log for UI frozen by using synchronous tasks etc.).

And as for the code complexity, I don't see much difference between:
var exists = app.filesystem.fileExists( fname);
or
var exists = await app.filesystem.fileExistsAsync( fname);

=> it is still just single line of code.

Assigned to Jiri to review and make the decision whether synchronous methods should be added.

zvezdan

2022-05-25 18:54

updater   ~0068260

It is not only problem with that line. It is problem because the line that follows it is not necessarily executed immediately after it.

I spend a lot of time experimenting to resolve that problem, just because of that asynchronous way of executing. If you want, I could send you the code which will give the false result because there are code executed asynchronously between await fileExistsAsync and the line that follows it.

I also don't see how these two variants are different in freezing of UI:
async main() {
  async function Test1(fname) {
    return await app.filesystem.fileExistsAsync(fname);
  }
  await Test1(fname);
}

and:
async main() {
  function Test2(fname) {
    return app.filesystem.fileExists(fname);
  }
  Test2(fname);
}

jiri

2022-05-25 20:42

administrator   ~0068263

The diff is that in the second case the whole main js thread is blocked for the duration of fileExists() method, i.e. UI appears to be frozen, while in the first case other JS code (events, etc.) can normally proceed. That's the reason why any method that in principle can take more than ~1ms or so must be async. Thanks to the 'await' syntax the resulting code isn't that much more complex or harder to read, so I'd say it's not that big deal.

The only part where longer processing time doesn't matter is in WebWorkers, which we don't utilize in MM yet, but maybe they'd be useful in some cases.