Skip to content

Conversation

@eerhardt
Copy link
Member

@eerhardt eerhardt commented Apr 1, 2020

Accesses to this list are not thread-safe because we can be enumerating it on one thread (in Return) while another thread adds to it (in Create).

The list is unnecessary because the PredictionEngine will always be "rooted". Either it was being held in memory by someone who got the PredictionEngine from the pool, or it is being held by the ObjectPool itself. So it won't ever be GC'd, and the WeakReference is not doing anything.

Also, inherit from PooledObjectPolicy instead of IPooledObjectPolicy, so it can take the "fastPolicy" path in DefaultObjectPool. (See dotnet/extensions#318).

Fix #4981

Accesses to this list are not thread-safe because we can be enumerating it on one thread (in Return) while another thread adds to it (in Create).

The list is unnecessary because the PredictionEngine will always be "rooted". Either it was being held in memory by someone who got the PredictionEngine from the pool, or it is being held by the ObjectPool itself. So it won't ever be GC'd, and the WeakReference is not doing anything.

Also, inherit from PooledObjectPolicy instead of IPooledObjectPolicy, so it can take the "fastPolicy" path in DefaultObjectPool. (See dotnet/extensions#318).

Fix dotnet#4981
@eerhardt eerhardt requested a review from antoniovs1029 April 1, 2020 19:08
@eerhardt eerhardt requested a review from a team as a code owner April 1, 2020 19:08
Copy link
Contributor

@antoniovs1029 antoniovs1029 left a comment

Choose a reason for hiding this comment

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

LGTM

@eerhardt eerhardt merged commit 54223a1 into dotnet:master Apr 2, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PredictionEnginePool.GetPredictionEngine is not thread safe

2 participants