Skip to content

Fix #1056: ensure boundary in TaskCreate if LCP enabled#1063

Merged
Avasil merged 1 commit into
monix:masterfrom
oleg-py:fix-taskcreate-local-isolation
Nov 7, 2019
Merged

Fix #1056: ensure boundary in TaskCreate if LCP enabled#1063
Avasil merged 1 commit into
monix:masterfrom
oleg-py:fix-taskcreate-local-isolation

Conversation

@oleg-py
Copy link
Copy Markdown
Collaborator

@oleg-py oleg-py commented Oct 25, 2019

The optimization bypassing the async boundary can break Local isolation.

The optimization bypassing the async boundary can break Local
isolation.
@oleg-py oleg-py requested review from Avasil and alexandru October 25, 2019 13:00
if (shouldPop) ctx.connection.pop()
// Optimization — if the callback was called on the same thread
// where it was created, then we are not going to fork
// This is not safe to do when localContextPropagation enabled
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it not safe?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this extends a TrampolinedRunnable, and TracingScheduler extends BatchingScheduler, doing execute bypasses the TracingRunnable creation and context isolation that it has.

Without that isolation, the continuation of that async/asyncF/etc. node therefore will have the context from whoever called the cb, not from where the async was actually created.

That only happened in case cb was called synchronously, but it does happen deep in ConcurrentChannel impl in certain concurrent usage patterns with fast producer and slow-ish consumer.

@Avasil Avasil merged commit 5f49fb6 into monix:master Nov 7, 2019
mdedetrich pushed a commit to mdedetrich/monix that referenced this pull request Mar 28, 2020
)

The optimization bypassing the async boundary can break Local
isolation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants