arphaman added a comment.

In https://reviews.llvm.org/D33357#761400, @bruno wrote:

> Any idea why we're hitting this issue in the first place? The error that gets 
> cleaned up is reported at some point before? Seems to me that we're going to 
> fail to update the timestamp but continue as nothing happened, I wonder how 
> many other issues this might trigger...


I don't know, I only have access to a crashlog without a reproducer.
Correct me if I'm wrong though, but judging by the way the timestamp is used, 
it seems to me that if we fail to update the timestamp nothing bad will happen, 
except the build will have to do more work as it will have to validate the 
system input files in that module for each CU until the first write to the 
module's timestamp succeeds.
I actually don't need the first change in the `if` it turns out, because that 
doesn't trigger a crash. The open file error is cleaned up and ignored, so I 
don't think that cleaning up an error on closure will break things.

>> I couldn't think of a way to test this, do you think it's possible to have a 
>> test for this?
> 
> If you can come up with a testcase it would be awesome, but for this to 
> trigger you'd have to reproduce the concurrent scenario in the testcase, 
> which I don't see how.

It seems that I can't really test a failure on write/close unfortunately.


Repository:
  rL LLVM

https://reviews.llvm.org/D33357



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to