ezet / evelib

Eve Online Library.NET is an open source C# wrapper for CCPs Eve Online API and other popular Eve Online APIs.
Apache License 2.0
72 stars 36 forks source link

EveCrest: Threading NullreferenceExcepion EveCrest #72

Open hrkrx opened 8 years ago

hrkrx commented 8 years ago

I tried to boost my requestspeed with threading and ran into the following error:

System.AggregateException wurde nicht behandelt.
  HResult=-2146233088
  Message= At least one error...
  Source=mscorlib
  StackTrace:
       at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
       at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
       at System.Threading.Tasks.Task`1.get_Result()
       at eZet.EveLib.EveCrestModule.EveCrest.Load[T](Href`1 uri, String[] parameters)
       at ~.Updater.<>c__DisplayClass0_0.<Update>b__3() in ~\Updater.cs: Line32
       at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
       at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
       at System.Threading.ThreadHelper.ThreadStart()
  InnerException: 
       HResult=-2147467261
       Message= Objectreference no instance and so on
       Source=EveLib.EveCrest
       StackTrace:
            at eZet.EveLib.EveCrestModule.RequestHandlers.CachedCrestRequestHandler.<RequestAsync>d__48`1.MoveNext()
            at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
            at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
            at eZet.EveLib.EveCrestModule.EveCrest.<requestAsync>d__91`1.MoveNext()
       InnerException: 

This may be a bug because when i run only one thread its working and if i put enough delay into my algorithm its also working (most of the time).

The code to reproduce this error:


        public void Update()
        {
            eZet.EveLib.EveCrestModule.EveCrest ec = new eZet.EveLib.EveCrestModule.EveCrest();
            ec.EnableRootCache = false;
            var allregions = ec.GetRoot().Query(r => r.Regions);
            foreach (var item in allregions.AllItems())
            {
                eZet.EveLib.EveCrestModule.Models.Links.Href<eZet.EveLib.EveCrestModule.Models.Resources.Region> regionHref = item.Href;
                new Thread(() =>
                {
                    Console.WriteLine("Starting Thread");
                    eZet.EveLib.EveCrestModule.EveCrest ecT = new eZet.EveLib.EveCrestModule.EveCrest();
                    ecT.EnableRootCache = false;

                   //ERROR spawning here
                    var region = ecT.Load(regionHref, new string[] { });
                    foreach (var cons in region.Constellations)
                    {
                        var constellation = ecT.Load<eZet.EveLib.EveCrestModule.Models.Resources.Constellation>(cons.Uri, new string[] { });
                        foreach (var sys in constellation.Systems)
                        {
                            var system = ecT.Load<eZet.EveLib.EveCrestModule.Models.Resources.SolarSystem>(sys.Uri, new string[] { });
                            foreach (var pl in system.Planets)
                            {
                                var planet = ecT.Load<eZet.EveLib.EveCrestModule.Models.Resources.Planet>(pl.Uri, new string[] { });
                            }
                        }
                    }
                }).Start();
            }
ezet commented 8 years ago

Using threads like that really won't make much of a difference, and the API you are using is very old, manually creating threads. Most of the time is spent waiting for HTTP responses, which is why the whole library is designed with TAP using asynchronous calls. To take advantage of that, you should use the Async versions of all the methods. If you want to use separate threads, you should probably at least use the TASK api, which makes things a great deal better, and I've had on issues with that. Also keep in mind that performing requests through several different EveCrest objects may cause you to pass the amount of requests allowed per second. If you use async, EveCrest spawns up to 20 threads for public and 10 for authenticated crest.

Another thing, you have a lot of obsolete code in your program. If you pass in the entity object instead of the entity url, you do not have to specify the type parameter for Load., and you do not have to pass in an empty parameter array, just leave it out.

          var allregions = ec.GetRoot().Query(r => r.Regions);
            foreach (var regionLink in allregions.AllItems()) {
                list.Add(new Thread(() => {
                    Console.WriteLine("Starting Thread");
                    EveCrest ecT = new EveCrest();
                    //ecT.EnableRootCache = false;

                    //ERROR spawning here
                    var region = ecT.Load(regionLink);
                    foreach (var cons in region.Constellations) {
                        var constellation = ecT.Load(
                            cons);
                        foreach (var sys in constellation.Systems) {
                            var system = ecT.Load(sys);
                            foreach (var pl in system.Planets) {
                                var planet = ecT.Load(pl);
                            }
                        }
                    }
                }));
            }
ezet commented 8 years ago

I am however not able to reproduce this bug, however, the hardware I'm running at isn't the fastest. In any case, I would first try to use async and possibly increase the threadpoolsize and see if that solves your problem. There are separate threadpool properties for public and authenticated crest on ec.RequestHandler.

hrkrx commented 8 years ago

It seems you are right and the threadpool is causing the exception, i will try running my code on different machines and report back if it really solves the error.

btw. thank you for helping me :)

ezet commented 8 years ago

Do you mind explaining how the threadpool is causing the exception? Also, do you use the async methods in your real code ? Let me know if you need any help with that. There shouldn't really be a need to use EveLib with threads like you did, but I'd still like to figure out what's causing problems and why. I realized it created a lot of IoExceptions because the cache file is busy, but I didn't see any Null Reference exceptions, and I'm not sure how they could happen.

On Sat, Feb 20, 2016 at 9:39 PM, hrkrx notifications@github.com wrote:

It seems you are right and the threadpool is causing the exception, i will try running my code on different machines and report back if it really solves the error.

btw. thank you for helping me :)

— Reply to this email directly or view it on GitHub https://github.com/ezet/evelib/issues/72#issuecomment-186678793.

hrkrx commented 8 years ago

Nope my assumption was wrong. At first i thought it might causing the error because when i changed the properties it worked, but only if the debugger wa attached. Without the debugger i get the same result.

To your second statement, in my experience if i dont use threads i get about 4 responses per second, with at least 5 threads i get up to 50 responses per second.

In my real code i use LoadAsync but it doesnt make a difference if i use nonasync (in terms of exceptionthrowing)

and why is the cache logic triggerd even if its set to false?