search

HPCE / hpce-2017-cw6

2 stars 17 forks source link

DeleteReadBin Concurrency #12

Open obrown opened 6 years ago

obrown commented 6 years ago

When operating on a file, SmallCrush calls ufile_CreateReadText but when it comes to cleaning up it calls ufile_DeleteReadBin. This function doesn't clear the co1 atomic flag which was set by ufile_CreateReadText. This causes an assertion error when a second batch of tests is run, for example, in search_std.

Replacing ufile_DeleteReadBin with ufile_DeleteReadText fixes the issue.

Shouldn't DeleteReadBin also clear the co2 flag?

https://github.com/HPCE/hpce-2017-cw6/blob/0034b313f8ba91d5681d53781a6afa667310b8a3/testu01/testu01/bbattery.cpp#L662

m8pple commented 6 years ago

I see what you mean - so yes, it looks like there are two separate bugs here from the original code-base:

In the original code the co1, co2 etc. were integers which were incremented and decremented, so there must have been --co2 missing in the original code.

This is more than a theoretical bug, and might trip people up as they explore the functions, so it definitely has potential impact within the scope of this coursework.

On the other hand it is relatively straightforward to diagnose, with the assertion highlighting what is going wrong. It's also only loosely a concurrency issue, as though it is related to thread-safety it is more a straight logic error.

So I'm going to tentatively assign a 0.5% bounty to this, so +0.2% overall - it's a bit meagre I know, but it applies to the group, so I need to be relatively careful about giving out too many marks and encouraging people to spend too much time bug hunting.