Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

ipfs-js can crash Firefox 57 #1089

Closed
interfect opened this issue Nov 19, 2017 · 14 comments
Closed

ipfs-js can crash Firefox 57 #1089

interfect opened this issue Nov 19, 2017 · 14 comments
Labels
kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up

Comments

@interfect
Copy link

interfect commented Nov 19, 2017

  • Version: v0.26.0 (whatever's deployed to https://unpkg.com/ipfs/dist/index.min.js as of right now)
  • Platform: Firefox 57.0 on Ubuntu 17.10 (4.13.0-16-generic #19-Ubuntu SMP Wed Oct 11 18:35:14 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux)
  • Subsystem: Unknown

Type: Bug

Severity: Critical (though maybe Mozilla's problem)

Description: Opening a tab that starts up an ipfs-js instance and leaving it for a few minutes will crash that tab (and all the other tabs in the browser).

Steps to reproduce the error:

  1. Download https://github.com/ipfs/js-ipfs/blob/master/examples/browser-script-tag/index.html
  2. Serve it behind a web server (e.g. npm's http-server). EDIT: You can also just go to https://ipfs.io/ipfs/QmYjSUG7bUaNDYybSdvcq5JZ1jUNdoNdCn4rC1rQCvge8u/
  3. Browse to the page in Firefox 57.0 on Ubuntu 17.10.
  4. Open the console (Ctrl+Shift+K). Not necessary.
  5. Wait. Browse around a bit in other tabs.
  6. After 2-5 minutes, you should have the page you are looking at disappear, and get a message from the browser that your tab has crashed.
@interfect
Copy link
Author

I've also reported this to Mozilla as https://bugzilla.mozilla.org/show_bug.cgi?id=1418736

@kevinsimper
Copy link

It also crashes the latest version of Chrome

@davidak
Copy link

davidak commented Nov 24, 2017

I can confirm it crashes in Firefox 57 and Chromium 62.

To test, just open: https://cdn.rawgit.com/ipfs/js-ipfs/93cc2739/examples/browser-script-tag/index.html

@neuthral
Copy link

I also had this problem with both 0.25.0 and later 0.26.0 on Vivaldi (1.13.1008.32 (Stable channel) (64-bit)) running on debian stretch,
I got it to not to crash when i removed all Bootstrap nodes from the config and swarms
{ init: true, start: true, repo: 'ipfs-browser', config: { "Addresses": { "Swarm": [ // "/dns4/star-signal.cloud.ipfs.team/wss/p2p-webrtc-star" ], "API": "", "Gateway": "" }, "Discovery": { "MDNS": { "Enabled": false, "Interval": 10 }, "webRTCStar": { "Enabled": false } } }, EXPERIMENTAL: { pubsub: true, sharding: true, dht: true } }

now it only connects to 7 peers as earlier i had 10-12, can this be a connection problem?

@interfect
Copy link
Author

Is this a duplicate of #950 maybe? I'll have to check the thread counts somehow.

@mitra42
Copy link

mitra42 commented Nov 26, 2017

@interfect its a duplicate of something that was prematurely closed IMHO.

@daviddias
Copy link
Member

@interfect v0.27.0 just got released which packs a lot of perf and mem improvements plus it disables WebRTC by default, you should not see any more crashes of this time.

We are working towards a solution in which using WebRTC doesn't mean that a browser will run out of memory, you can track that conversation here: #1088 (comment)

Meanwhile, if you want to enable WebRTC again or try a new Relayed transport (websocket-star), check out the FAQ https://github.com/ipfs/js-ipfs#faq

@mitra42
Copy link

mitra42 commented Dec 4, 2017

@diasdavid - I'm just testing the websocket-star interface, previously we had to do EXPERIMENTAL: { pubsub: true } in order to make YJS work, I'm not sure if that was tied to the WebRTC transport, should we leave that true for websocket-star or remove it?

@kevinsimper
Copy link

@daviddias
Copy link
Member

daviddias commented Dec 5, 2017

@mitra42 yes, you should keep the PubSub flag to true. PubSub is experimental for the whole IPFS protocol, both go-ipfs and js-ipfs and so it is hidden behind a flag. PubSub is transport agnostic (as in all things libp2p).

@kevinsimper let me know how it works for you! :)

@mitra42
Copy link

mitra42 commented Dec 5, 2017

"Yes you should' means which choice ( it was an a,b choice, not yes/no) :-) a: Leave true or b: Remove

If pubsub is transport agnostic, I would assume that we still need it for YJS ?

@daviddias
Copy link
Member

Updated my comment. Yes, you should keep the PubSub flag to true.

@mitra42
Copy link

mitra42 commented Dec 5, 2017

Great - and that looks to have cracked the problem for our apps (Yjs). I left it running for an hour or so though I did have a browser crash, but not 100% certain it was IPFS (it might have been something else I was testing) so re-running to make sure .

@daviddias daviddias added kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up labels Dec 11, 2017
@daviddias
Copy link
Member

Excellent! I'm closing this issue then. Thank you for testing it out! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

No branches or pull requests

6 participants