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

Added implementation. #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

AraHaan
Copy link

@AraHaan AraHaan commented Aug 29, 2022

Public Types:

  • MutableServiceProvider
  • MutableServiceProviderFactory

@CLAassistant
Copy link

CLAassistant commented Aug 29, 2022

CLA assistant check
All committers have signed the CLA.

@Foxtrek64
Copy link
Member

GitHub Actions has been set up. I'll review here shortly.

@AraHaan
Copy link
Author

AraHaan commented Aug 29, 2022

Ah ok.

Public Types:
- MutableServiceProvider
- MutableServiceProviderFactory

Signed-off-by: AraHaan <[email protected]>
@AraHaan
Copy link
Author

AraHaan commented Aug 29, 2022

Alright, these changes might require git config --system core.longpaths true to be set.

As well as this: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=cmd#enable-long-paths-in-windows-10-version-1607-and-later

@AraHaan
Copy link
Author

AraHaan commented Aug 30, 2022

Alright, I tested it locally with my Discord bot with Remora.Plugins using this code and for some reason it crashes with an resource load exception.

Remora.Plugins now works with this.
@AraHaan
Copy link
Author

AraHaan commented Sep 1, 2022

I am not sure what happens in Remora/Remora.Discord#231 but perhaps CallSiteFactory tries to restart all Singleton instances every time new ServiceCollections are added?

@AraHaan
Copy link
Author

AraHaan commented Sep 1, 2022

Issue in Remora.Discord turns out to have been fixed there (ironically with this it now runs as if the code was for a sharded bot).

Comment on lines +314 to +328
var serviceCount = _serviceCollections.CountServices();
for (int i = serviceCount.Count - 1; i >= 0; i--)
{
var descriptor = serviceCount[i];
var callSite = TryCreateExact(descriptor, itemType, callSiteChain, slot) ??
TryCreateOpenGeneric(descriptor, itemType, callSiteChain, slot, false);

if (callSite != null)
{
slot++;

cacheLocation = GetCommonCacheLocation(cacheLocation, callSite.Cache.Location);
callSites.Add(callSite);
}
}
Copy link
Author

Choose a reason for hiding this comment

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

It seems that every time a service collection is added, that then new callsites are created (even ones that already existed) which results in new instances being created. Oops.

@AraHaan
Copy link
Author

AraHaan commented Sep 3, 2022

Welp I think this is a failure somewhat, In terms of singleton instances getting restarted it seems whenever service collections are added/removed from the CallSiteFactory.

The sad part is that code is complex as well making it hard to figure out a way to not have that happen.

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.

3 participants