search

powersync-ja / powersync-js

SDK that enables local-first and real-time reactive apps with embedded SQLite for JavaScript clients, including React Native and Web
https://www.powersync.com
Apache License 2.0
259 stars 13 forks source link

Memory leak when `transaction.complete()` is not awaited #247

Closed byzyk closed 1 month ago

byzyk commented 1 month ago

Failure to call await transaction.complete() inside uploadData() results in an endless loop and eventually hangs the browser.

I understand that it's not intended behavior, so I would appreciate it if we could find a way to detect the memory leak and throw an exception instead. It would potentially save many dev hours for those who'll make the same mistake as I just did 😃

PS. Amazing library, I enjoy it quite a lot the past few days!

stevensJourney commented 1 month ago

Thanks for logging this issue. We'll investigate by reproducing on our side.

stevensJourney commented 1 month ago

Hi @byzyk, I tried to reproduce a leak and browser hang on my side by removing this line in our demo.

I did notice that the upload function remains in a non-blocking loop, but I didn't notice indefinite increasing memory usage or a hang. Attached below is the profile of memory usage which did remain stable. image

We are still investigating ways to potentially detect if the transaction is not completed and avoid the loop. It would be helpful if you could share some more context on how the hang or leak occurred on your side.

stevensJourney commented 1 month ago

We've added some additional checks in these versions https://github.com/powersync-ja/powersync-js/pull/255 to delay infinite loops if no uploads are performed in the uploadData method of the BackendConnector. This should help a lot with these kinds of issues. Generally an infinite loop could cause large memory spikes depending on what the client code is doing in uploadData. I'm going to close this for now since I could not reproduce a memory leak on my side.

byzyk commented 3 weeks ago

hi @stevensJourney, thanks for the update.

I just wanted to clarify from my side - you are correct, what was happening here was an infinite loop. In my case, I had a memory leak probably due to some additional logic inside uploadData. Thanks for patching this!