AutomaApp / automa

A browser extension for automating your browser by connecting blocks
https://www.automa.site
Other
10.78k stars 1.1k forks source link

JavaScript Code can cause serious memory issues. #1553

Closed slavi010 closed 7 months ago

slavi010 commented 7 months ago

Describe the bug JavaScript Code can cause serious memory issues (with the function automaRefData for example).

Its like the data inside aren't "garbaged" at the end of the JavaScript Code block (they are, at least, at the end of the workflow).

To Reproduce Have at least 2 blocks: One for inserting some data (tested with a string.length ~= 200 000 caracters) to a variable. Let call it 'test'. The other one to get the 'test' variable via the function automaRefData. Loop the JavaScript Code block.

Expected behavior No memory leak.

Screenshots image

image

image

Desktop (please complete the following information):

Additional context Maybe I just doing something wrong somewhere ? ^^'

Btw, thanks a lot for your help :D

v-smwang commented 7 months ago

Yes, I also have memory problems here, I did not test it separately, my workflow is more complicated, but I feel that automaRefData and automaSetData have memory problems, and the use of indexedDB also feels that there are memory problems.

v-smwang commented 7 months ago

This problem will cause the page to crash if it runs longer

slavi010 commented 7 months ago

Well, I did a bit of more test and found super really wierd things like this:

image => No memory issue

image => Memory issue

image => No memory issue

Siykt commented 7 months ago

The 'state' is storing too many logs.

image image

after I stop writing to refDataSnapshot, the memory consumption of jsblock can be basically ignored.

v-smwang commented 7 months ago

Turn off the log and run it again. I'm testing it too

v-smwang commented 7 months ago

The 'state' is storing too many logs.

image image

and jsblock not clean:

image

Do you mean that every time the jsblock variable in the loop is not cleaned up, it keeps accumulating?

Siykt commented 7 months ago

There are two issues now:

  1. refDataSnapshots is storing too many variables.
  2. Logs.

image

v-smwang commented 7 months ago

Yes, even if logging is turned off, snapshots of variables are still stored. It should be designed to be easy to find problems. It is normal to close the log, but the shutdown should also close the data snapshot, which is not done.

v-smwang commented 7 months ago

Yes, even if logging is turned off, snapshots of variables are still stored. It should be designed to be easy to find problems. It is normal to close the log, but the shutdown should also close the data snapshot, which is not done.

@Siykt Am I right?

Siykt commented 7 months ago

this:

src/workflowEngine/WorkflowEngine.js:309


addRefDataSnapshot(key) {
this.refDataSnapshotsKeys[key].index += 1;
this.refDataSnapshotsKeys[
key
].key = `##${key}${this.refDataSnapshotsKeys[key].index}`;
const keyName = this.refDataSnapshotsKeys[key].key;
this.refDataSnapshots[keyName] = cloneDeep(this.referenceData[key]);

}



I think this is mainly used for debugging, because the logger actually stores directly into the `browser.storage`.

![image](https://github.com/AutomaApp/automa/assets/44022526/b14bd0ba-6039-4e10-8eae-d3c2f000b661)
v-smwang commented 7 months ago

@Siykt #1556 Have any opinion on this problem?I'd like to discuss it with you

Siykt commented 7 months ago

I found that refDataSnapshots is completely a bunch of redundant data. And in debugMode, it only uses the last referenceData.

image

Kholid060 commented 7 months ago

The cause of this issue may be the browser.runtime.sendMessage method that Automa used to send the variables data from background script to the content script. This method needs to serialize the data which might cause a memory spike especially when the variables have large values.

Siykt commented 7 months ago

@Kholid060 jsblock Execution context is background:

image