Address re-entrancy when playing ops (#3242) #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When callback is called, we immediately process op synchronously, resulting in re-entrancy and eventually running out of stack (if processing too many ops). This problem was avoided in old code path due to using continuations.
A bunch of merges, not 100% they are all correct.
And remove insertPhoto/insertVideo/urlToInclusion
This was referenced Nov 21, 2019
This was referenced Sep 2, 2020
Merged
jatgarg
pushed a commit
to jatgarg/FluidFramework-1
that referenced
this pull request
Aug 25, 2022
Add scripts for scale test automation
curtisman
added a commit
that referenced
this pull request
Oct 15, 2022
andrewstanovsky
added a commit
that referenced
this pull request
Jan 31, 2023
NicholasCouri
added a commit
that referenced
this pull request
Mar 28, 2023
## Description Currently tinylicious fails to run sample scenarios in the [Share Live Repo](https://github.com/microsoft/live-share-sdk) 2 interesting commits: a) [Update deps (minor) on server release group in tinylicious (#14586) · 12b3f4c (github.com)](12b3f4c#diff-18fce4d66a0b908f502bc635b1c1377557fd7ec4f8d9df8dcaa4d9c261158ca9)) Throws when getTenantGitManager is invoked. 2) [Separate api for getting tenant gitManager. (#14290) · d368f26 (github.com)](d368f26#diff-deeaa88a2f939b81094f7412e1482709a59c4c47850a5c379c92aef0b78d70d5)) Moved the call when creating the document from getTenant to getTenantGitManager and I'm not sure if it applies to tinylicious but it is consumed by it. This PR is trying to revert the current behavior by keeping changes #2 and not throwing on #1 Repro steps: a) Clone https://github.com/microsoft/live-share-sdk b) Follow instructions from the Readme to install (I had to also install vite as it was failing without it) c) Simply try to run one of the samples (I'm using samples\javascript\03.live-canvas-demo\) http://localhost:3000/
NicholasCouri
added a commit
to NicholasCouri/FluidFramework
that referenced
this pull request
Mar 28, 2023
## Description Currently tinylicious fails to run sample scenarios in the [Share Live Repo](https://github.com/microsoft/live-share-sdk) 2 interesting commits: a) [Update deps (minor) on server release group in tinylicious (microsoft#14586) · microsoft/FluidFramework@12b3f4c (github.com)](microsoft@12b3f4c#diff-18fce4d66a0b908f502bc635b1c1377557fd7ec4f8d9df8dcaa4d9c261158ca9)) Throws when getTenantGitManager is invoked. 2) [Separate api for getting tenant gitManager. (microsoft#14290) · microsoft/FluidFramework@d368f26 (github.com)](microsoft@d368f26#diff-deeaa88a2f939b81094f7412e1482709a59c4c47850a5c379c92aef0b78d70d5)) Moved the call when creating the document from getTenant to getTenantGitManager and I'm not sure if it applies to tinylicious but it is consumed by it. This PR is trying to revert the current behavior by keeping changes microsoft#2 and not throwing on microsoft#1 Repro steps: a) Clone https://github.com/microsoft/live-share-sdk b) Follow instructions from the Readme to install (I had to also install vite as it was failing without it) c) Simply try to run one of the samples (I'm using samples\javascript\03.live-canvas-demo\) http://localhost:3000/
This was referenced Mar 28, 2023
NicholasCouri
added a commit
that referenced
this pull request
Mar 29, 2023
## Description Currently tinylicious fails to run sample scenarios in the [Share Live Repo](https://github.com/microsoft/live-share-sdk) 2 interesting commits: a) [Update deps (minor) on server release group in tinylicious (#14586) · 12b3f4c (github.com)](12b3f4c#diff-18fce4d66a0b908f502bc635b1c1377557fd7ec4f8d9df8dcaa4d9c261158ca9)) Throws when getTenantGitManager is invoked. 2) [Separate api for getting tenant gitManager. (#14290) · d368f26 (github.com)](d368f26#diff-deeaa88a2f939b81094f7412e1482709a59c4c47850a5c379c92aef0b78d70d5)) Moved the call when creating the document from getTenant to getTenantGitManager and I'm not sure if it applies to tinylicious but it is consumed by it. This PR is trying to revert the current behavior by keeping changes #2 and not throwing on #1 Repro steps: a) Clone https://github.com/microsoft/live-share-sdk b) Follow instructions from the Readme to install (I had to also install vite as it was failing without it) c) Simply try to run one of the samples (I'm using samples\javascript\03.live-canvas-demo\) http://localhost:3000/
andrewstanovsky
added a commit
that referenced
this pull request
Mar 29, 2023
andrewstanovsky
added a commit
that referenced
this pull request
May 3, 2023
andrewstanovsky
added a commit
that referenced
this pull request
Oct 11, 2023
andrewstanovsky
added a commit
to andrewstanovsky/FluidFramework
that referenced
this pull request
Oct 18, 2023
Closed
vladsud
pushed a commit
that referenced
this pull request
Feb 19, 2024
…esTest Remove the describeCompat from UT
vladsud
added a commit
that referenced
this pull request
Apr 16, 2024
…re, runtime, Loader in versioned tests (#20650) An extraction from #20621 Reason for changes These tests explode when I change interface between DDS & DataStore Description These tests are not written correctly. They mix arbitrary version of DDS with the latest version of FluidDataStoreRuntime (through TestFluidObjectFactory). We never supported such mixes - version of DDS has to match version or data store! Using matching TestFluidObjectFactory is easy, but then we start running into somewhat similar problem of mixing FluidDataStoreRuntime & ContainerRuntime. We support only N/N-1 mixes here, and nothing more. So we have to use the version provided by test framework, thus you see some dance above CodeLoader to get right factory. And while we are at it, it would be great for the test to use Loader version provided by test framework, not latest version. This breaks for old versions as the way test is written it works only for latest structure of the document. Previously test framework was running a ton of combos, but most of them were actually the same, as even when it was supplied different versions of loader, we were using only latest version of loader. So, the right fix here - use loader version provided by framework but skip all the tests that use old version of the loader, thus getting same result, but with fewer iterations (and faster tests). As part of fixing it, use proper back-compat compatible ways to get to entry points at loader & runtime layers.
CraigMacomber
added a commit
that referenced
this pull request
May 10, 2024
## Description This makes 3 changes to Tree.is: 1. calling Tree.is(node, SchemaNotUsedInTreeSchema) is no longer an error. This undocumented edge case could have been problematic, and is inconsistent since it would not fire if node was not a TreeNode. 2. Tree.is now takes in ImplicitAllowedTypes, making cases like Tree.is(x, [schema.number, schema.string]) valid. THis is more performant and more concise then doing two separate checks and ORing them together. This also allows checking a node against an ImplicitAllowedTypes pulled from a FieldSchema which could be handy for some generic code. 3. The implementation of Tree.schema and Tree.is have been rewritten to not rely on flex-schema as much, and fast path non-TreeNode inputs. This should make it more maintainable and more performant. Interestingly `#2` above is the only case that couldn't be covered by `instanceof` (assuming TypeScript 5.3): we could make `instanceof` do all narrowing currently done with Tree.is, except for this new case. The presence of this case thus seems to motivate keeping `Tree.is` if for no reason other than it can support this additional pattern which `instancof` cannot.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry pick [5bd8ac2]
When callback is called, we immediately process op synchronously, resulting in re-entrancy and eventually running out of stack (if processing too many ops).
This problem was avoided in old code path due to using continuations.