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

fixed owner null bug #30

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions src/TinyPubSub/TinyPubSub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public static class TinyPubSub
{
private static readonly ConcurrentDictionary<string, ConcurrentDictionary<ISubscription, ISubscription>> Channels = new ConcurrentDictionary<string, ConcurrentDictionary<ISubscription, ISubscription>>();
private static int order;

private static object genericOwner = new {};

private static ConcurrentDictionary<ISubscription, ISubscription> GetOrCreateChannel(string channel)
{
Expand All @@ -61,7 +63,12 @@ private static ISubscription CreateAndAdd(object owner, string channel, Action a
private static ISubscription CreateAndAdd<T>(object owner, string channel, Action<T> action, bool disableAfterFirstUse = false)
{
Interlocked.Increment(ref order);


if (owner == null)
{
owner = genericOwner;
}

var current = GetOrCreateChannel(channel);
var subscription = new Subscription<T>(owner, action, disableAfterFirstUse, order);
current.TryAdd(subscription, subscription);
Expand All @@ -71,7 +78,12 @@ private static ISubscription CreateAndAdd<T>(object owner, string channel, Actio
private static ISubscription CreateAndAdd<T>(object owner, string channel, Action<T, TinyEventArgs> action, bool disableAfterFirstUse = false)
{
Interlocked.Increment(ref order);


if(owner == null)
{
owner = genericOwner;
}

var current = GetOrCreateChannel(channel);
var subscription = new Subscription<T>(owner, action, disableAfterFirstUse, order);
current.TryAdd(subscription, subscription);
Expand All @@ -91,7 +103,7 @@ private static ISubscription CreateAndAdd<T>(object owner, string channel, Actio
/// <returns>A tag that can be used to unsubscribe.</returns>
public static string Subscribe(string channel, Action action)
{
var subscription = CreateAndAdd(null, channel, action);
var subscription = CreateAndAdd(genericOwner, channel, action);
return subscription.Tag;
}

Expand All @@ -103,7 +115,7 @@ public static string Subscribe(string channel, Action action)
/// <returns>A tag that can be used to unsubscribe.</returns>
public static string Subscribe(string channel, Action<string> action)
{
var subscription = CreateAndAdd<string>(null, channel, action);
var subscription = CreateAndAdd<string>(genericOwner, channel, action);
return subscription.Tag;
}

Expand Down Expand Up @@ -146,7 +158,7 @@ public static string Subscribe(object owner, string channel, Action<string> acti
/// <typeparam name="T">The type to subscribe to.</typeparam>
public static string Subscribe<T>(string channel, Action<T> action)
{
var subscription = CreateAndAdd<T>(null, channel, action);
var subscription = CreateAndAdd<T>(genericOwner, channel, action);
return subscription.Tag;
}

Expand All @@ -158,7 +170,7 @@ public static string Subscribe<T>(string channel, Action<T> action)
/// <param name="action">The action to execute.</param>
public static string Subscribe(string channel, Action<string, TinyEventArgs> action)
{
var subscription = CreateAndAdd(null, channel, action);
var subscription = CreateAndAdd(Channels, channel, action);
return subscription.Tag;
}

Expand Down Expand Up @@ -199,7 +211,7 @@ public static string Subscribe<T>(object owner, string channel, Action<T, TinyEv
/// <typeparam name="T">The type to subscribe to.</typeparam>
public static string Subscribe<T>(string channel, Action<T, TinyEventArgs> action)
{
var subscription = CreateAndAdd<T>(null, channel, action);
var subscription = CreateAndAdd<T>(genericOwner, channel, action);
return subscription.Tag;
}

Expand Down Expand Up @@ -334,6 +346,7 @@ public static TinyEventArgs PublishControlled<T>(string channel, T instance, Act
if (subscription.Owner.Target == null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why doesn't just

subscriber.Owner?.Target == null

be enough? Instead of creating a static object? Just trying to understand.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, then it will think the owner is collected by GC. Maybe we should deprecate the method of not having an owner?

{
subscriptionsToRemove.Add(subscription);
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why continue here? What happens if ActionWithArgumentAndArgs isn't null? Then it will not be called?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because if it is null the object owning it is been collected by GC. And then we can remove that subscription

}

if (subscription.ActionWithArgumentAndArgs != null)
Expand Down