fjoppe / Legivel

F# Yaml 1.2 parser
https://fjoppe.github.io/Legivel
The Unlicense
60 stars 5 forks source link

Concurrency issue #15

Open pkese opened 5 years ago

pkese commented 5 years ago

Hi,

After spliting my Legivel-touching test code into multiple files, I have found out that I'm getting some inconsistent error traces originating from Legivel code when multiple tests are run in parallel (in xUnit).
I only reach this situation when multiple tests are run in parallel (when manually running a single test at a time, no errors are detected).

Here is an example stack trace:

System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at Legivel.Parser.Memoize@595.Invoke(Tuple`3 k, a c)
   at Legivel.Parser.processFunc@1416-3.Invoke(ParseState ps)
   at Legivel.Parser.parseAndCache@449(ParsedCache this, NodeKind t, ParseState ps, FSharpFunc`2 parseFunc, Context c, Unit unitVar0)
   at Legivel.Parser.Yaml12Parser.c-single-quoted(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.c-flow-json-content(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.c-flow-json-node(ParseState ps)
   at Legivel.Parser.Yaml12Parser.c-s-implicit-json-key(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.ns-s-block-map-implicit-key(ParseState ps)
   at Legivel.Parser.Yaml12Parser.ns-l-block-map-implicit-entry(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.ns-l-block-map-entry(ParseState ps)
   at Legivel.Parser.l+block-mapping@2559(Yaml12Parser this, ParseState ps, ParseState psp, FSharpList`1 acc)
   at Legivel.Parser.parseAndCache@449(ParsedCache this, NodeKind t, ParseState ps, FSharpFunc`2 parseFunc, Context c, Unit unitVar0)
   at Legivel.Parser.Yaml12Parser.l+block-mapping(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.seq or map@2821.Invoke(ParseState pls)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.s-l+block-collection(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.s-l+block-in-block(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.s-l+block-node(ParseState ps)
   at Legivel.Parser.Yaml12Parser.l-bare-document(ParseState ps)
   at Legivel.Internals.ParserMonads.EitherBuilder`3.Either[a](EitherResult`2 pv, FSharpFunc`2 nw)
   at Legivel.Parser.Yaml12Parser.l-any-document(ParseState ps)
   at Legivel.Parser.l-yaml-stream@2963.Invoke(ParseState psr)
   at Legivel.Parser.Yaml12Parser.l-yaml-stream(String input)
   at Legivel.Customization.Mapping.ParseYamlToNative[tp](FSharpFunc`2 mapToNative, GlobalTagSchema schema, String yml)
   at Legivel.Serialization.DeserializeWithOptions[tp](FSharpList`1 options, String yaml)

Btw, my deserialization options are

[ MappingMode MapYaml.AndRequireFullProjection ]

and i'm using Open.Collections.OrderedDictionary in my models (place of F#'s Map).

I'll try to reduce to a reproducible example, but it might take me some time to remove all of my unnecessary code (for the moment I'm just running tests manually, or may introduce a lock around the loader).

I'm just writing this in case it gives you enough information to let you find a solution before I dig in. Otherwise I'll follow back later.

Thanks

pkese commented 5 years ago

Indeed, when I wrap the DeserializeWithOptions with a lock, the error goes away.

fjoppe commented 5 years ago

I’ll look into this, but perhaps a bit later. It appears as if Legivel isn’t as thread-safe as I hoped. I took some shortcuts for performance. But I’m currently working on a large perf improvement, so maybe I can discard/improve it in that feature.

Verstuurd vanaf mijn iPhone

Op 26 sep. 2019 om 14:51 heeft Peter Keše notifications@github.com<mailto:notifications@github.com> het volgende geschreven:

Indeed, when I wrap the DeserializeWithOptions with a lock, the error goes away.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/fjoppe/Legivel/issues/15?email_source=notifications&email_token=ACMMW5AONBRUO4JXRJUUWR3QLSV3FA5CNFSM4I2ZODDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7VOIDI#issuecomment-535487501, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACMMW5FSTVCTJ7CMMH7VHODQLSV3FANCNFSM4I2ZODDA.

pkese commented 5 years ago

Ok, in that case, I'd suggest that we wait until the new branch gets published and I try again then without my mutexes. For the time being the locks are not critical, because they only affect my test code.

fjoppe commented 5 years ago

Work is being done in this branch, it's my fourth try in getting it faster :). I'll get back on this. The idea was to cache various stuff that can be shared between different parse cycles.