pytest-dev / apipkg

MIT License
55 stars 17 forks source link

Fix race conditions for attribute creation #25

Closed GlenWalker closed 2 years ago

GlenWalker commented 2 years ago
RonnyPfannschmidt commented 2 years ago

I grasp the issue and recommend a slightly different approach

Instead of taking our on first access, it ough to be replaced with a threading. Event in a short locked block, the same should be done for the values that get taken and replaced

Then other threads may just wait for those to resolve and a Rlock is not needed

As only a single place is using the lock which is global, its OK to make it a private global variable

GlenWalker commented 2 years ago

Will have a go at alternate implementation

GlenWalker commented 2 years ago

Ok, had a first attempt at suggested implementation:

RonnyPfannschmidt commented 2 years ago

then back to the initial solution it is, sorry for that one,

GlenWalker commented 2 years ago

then back to the initial solution it is, sorry for that one,

It was definitely worth a try. My first attempt at this problem tried to use __setattr__ to track which attributes, if any, had been set during __onfirstaccess__, but the code for that also ended up more complicated than I was happy with.

Is there anything else you'd like changed, or is the current state of this PR something that you'd be happy to merge?

GlenWalker commented 2 years ago

I haven't added anything to the changelog for this - its missing an entry for 2.0, but I haven't had time to figure out what the changes were. I'm guessing we'd make this 2.1.0 or 2.0.1, apart from fixing the race conditions all other behaviour should be the same as 2.0.0

RonnyPfannschmidt commented 2 years ago

thanks!