Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken feature rendered #2547

Closed
cmdcolin opened this issue Nov 24, 2021 · 3 comments · Fixed by #2551
Closed

Broken feature rendered #2547

cmdcolin opened this issue Nov 24, 2021 · 3 comments · Fixed by #2551
Labels
bug Something isn't working

Comments

@cmdcolin
Copy link
Collaborator

Screenshot from 2021-11-24 10-56-13

Found on desktop, on current main. Not existing on v1.5.1. This is a .jbrowse file that might help (gzipped)

broken_feat.jbrowse.gz

On desktop

  • no bug on v1.5.1
  • bug on main

On web, afaict?

  • no bug on v1.5.1
  • no bug on main(or hard to reproduce?)

https://jbrowse.org/code/jb2/main/?session=share-4PvZipGrZA&password=QRu7V

https://jbrowse.org/code/jb2/v1.5.1/?session=share-4PvZipGrZA&password=QRu7V

@cmdcolin cmdcolin added the bug Something isn't working label Nov 24, 2021
@rbuels
Copy link
Contributor

rbuels commented Nov 24, 2021

This might be caused by GranularRectLayout going down this code path:

const additionalLength = oRight + 1
if (this.rowState.bits.length + additionalLength > this.widthLimit) {
console.warn(
'Layout width limit exceeded, discarding old layout. Please be more careful about discarding unused blocks.',
)
this.rowState = this.initialize(left, right)

When the layout gets too big, it re-initializes itself so we don't run out of memory. This can sometimes break the layout.

If that's what is causing this, then the fix is to explicitly garbage-collect the layout by calling layout.discardRange when a track block is disposed. We are not currently doing that.

@cmdcolin
Copy link
Collaborator Author

there is no console.warn so I don't think it is this code path

Screenshot from 2021-11-24 12-12-33

@cmdcolin
Copy link
Collaborator Author

The git bisect points to this commit as being the cause if I did it correctly

❯❯ git bisect bad
2bb304365795972b1ea9129ac4edf091c6f312f5 is the first bad commit
commit 2bb304365795972b1ea9129ac4edf091c6f312f5
Author: Garrett Stevens <stevens.garrett.j@gmail.com>
Date:   Tue Nov 2 15:48:05 2021 -0600

    Add option for plugin to be ESM or CJS module

 packages/core/PluginLoader.ts                      | 221 +++++++++++++++++----
 packages/core/package.json                         |   1 +
 packages/core/rpc/RpcManager.ts                    |  21 +-
 packages/core/rpc/WebWorkerRpcDriver.ts            |   4 +-
 plugins/data-management/package.json               |   1 +
 .../components/CustomPluginForm.tsx                | 194 ++++++++++++++----
 .../components/InstalledPlugin.tsx                 |   6 +-
 .../components/PluginStoreWidget.test.js           |  12 +-
 products/jbrowse-desktop/public/electron.ts        |   4 +
 products/jbrowse-desktop/src/StartScreen/util.tsx  |   7 +-
 products/jbrowse-desktop/src/jbrowseModel.ts       |  18 +-
 products/jbrowse-desktop/src/rootModel.ts          |   1 +
 products/jbrowse-web/src/Loader.tsx                |  48 ++++-
 products/jbrowse-web/src/jbrowseModel.ts           |  18 +-
 14 files changed, 448 insertions(+), 108 deletions(-)

This may correspond with what I'm seeing a little bit where I see BaseRpcDriver->constructor is getting called multiple times

CC @garrettjstevens if you got any ideas...i see this.getDriverForCall is now async, maybe this could be allowing for race condition?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants