{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/17330","repository_url":"https://api.github.com/repos/dotnet/coreclr","labels_url":"https://api.github.com/repos/dotnet/coreclr/issues/17330/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/17330/comments","events_url":"https://api.github.com/repos/dotnet/coreclr/issues/17330/events","html_url":"https://github.com/dotnet/coreclr/pull/17330","id":309919398,"number":17330,"title":"GCStress: try to reduce races and tolerate races better","user":{"login":"AndyAyersMS","id":10121823,"avatar_url":"https://avatars1.githubusercontent.com/u/10121823?v=4","gravatar_id":"","url":"https://api.github.com/users/AndyAyersMS","html_url":"https://github.com/AndyAyersMS","followers_url":"https://api.github.com/users/AndyAyersMS/followers","following_url":"https://api.github.com/users/AndyAyersMS/following{/other_user}","gists_url":"https://api.github.com/users/AndyAyersMS/gists{/gist_id}","starred_url":"https://api.github.com/users/AndyAyersMS/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AndyAyersMS/subscriptions","organizations_url":"https://api.github.com/users/AndyAyersMS/orgs","repos_url":"https://api.github.com/users/AndyAyersMS/repos","events_url":"https://api.github.com/users/AndyAyersMS/events{/privacy}","received_events_url":"https://api.github.com/users/AndyAyersMS/received_events","type":"User","site_admin":false},"labels":[{"id":314279199,"url":"https://api.github.com/repos/dotnet/coreclr/labels/GCStress","name":"GCStress","color":"5319e7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/dotnet/coreclr/milestones/18","html_url":"https://github.com/dotnet/coreclr/milestone/18","labels_url":"https://api.github.com/repos/dotnet/coreclr/milestones/18/labels","id":3246413,"number":18,"title":"2.2.0","description":"Placeholder name for next major release after 2.1. [Official release dates](https://github.com/dotnet/core/blob/master/roadmap.md) are TBD.\r\n\r\nWe will move important issues from Future to 2.2.0 as the priorities and dates for next release become more clear.","creator":{"login":"karelz","id":5776068,"avatar_url":"https://avatars2.githubusercontent.com/u/5776068?v=4","gravatar_id":"","url":"https://api.github.com/users/karelz","html_url":"https://github.com/karelz","followers_url":"https://api.github.com/users/karelz/followers","following_url":"https://api.github.com/users/karelz/following{/other_user}","gists_url":"https://api.github.com/users/karelz/gists{/gist_id}","starred_url":"https://api.github.com/users/karelz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/karelz/subscriptions","organizations_url":"https://api.github.com/users/karelz/orgs","repos_url":"https://api.github.com/users/karelz/repos","events_url":"https://api.github.com/users/karelz/events{/privacy}","received_events_url":"https://api.github.com/users/karelz/received_events","type":"User","site_admin":false},"open_issues":41,"closed_issues":8,"state":"open","created_at":"2018-04-05T22:09:22Z","updated_at":"2018-04-18T16:38:47Z","due_on":null,"closed_at":null},"comments":48,"created_at":"2018-03-29T22:08:27Z","updated_at":"2018-04-18T16:57:28Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/17330","html_url":"https://github.com/dotnet/coreclr/pull/17330","diff_url":"https://github.com/dotnet/coreclr/pull/17330.diff","patch_url":"https://github.com/dotnet/coreclr/pull/17330.patch"},"body":"This change addresses races that cause spurious failures in when running\r\nGC stress on multithreaded applications.\r\n\r\n* Instruction update race\r\n\r\nThreads that hit a gc cover interrupt where gc is not safe can race to\r\noverrwrite the interrupt instruction and change it back to the original\r\ninstruction.\r\n\r\nThis can cause confusion when handling stress exceptions as the exception code\r\nraised by the kernel may be determined by disassembling the instruction that\r\ncaused the fault, and this instruction may now change between the time the\r\nfault is raised and the instruction is disassembled. When this happens the\r\nkernel may report an ACCESS_VIOLATION where there was actually an attempt to\r\nexecute a priveledged instruction.\r\n\r\nx86 already had a tolerance mechanism here where when gc stress was active\r\nand the exception status was ACCESS_VIOLATION the faulting instruction would\r\nbe retried to see if it faults the same way again. In this change we extend\r\nthis to tolerance to cover x64 and also enable it regardless of the gc mode.\r\nWe use the exception information to further screen as these spurious AVs look\r\nlike reads from address 0xFF..FF.\r\n\r\n* Instrumentation vs execution race\r\n\r\nThe second race happens when one thread is jitting a method and another is\r\nabout to call the method. The first thread finishes jitting and publishes the\r\nmethod code, then starts instrumenting the method for gc coverage. While this\r\ninstrumentation is ongoing, the second thread then calls the method and hits\r\na gc interrupt instruction. The code that recognizes the fault as a gc coverage\r\ninterrupt gets confused as the instrumentation is not yet complete -- in\r\nparticular the m_GcCover member of the MethodDesc is not yet set. So the second\r\nthread triggers an assert.\r\n\r\nThe fix for this is to instrument for GcCoverage before publishing the code.\r\nSince multiple threads can be jitting a method concurrently the instrument and\r\npublic steps are done under a lock to ensure that the instrumentation and code\r\nare consistent (come from the same thread).\r\n\r\nWith this lock in place we have removed the secondary locking done in\r\nSetupGcCoverage as it is no longer needed; only one thread can be instrumenting\r\na given jitted method for GcCoverage.\r\n\r\n* Instruction cache flushes\r\n\r\nIn some cases when replacing the interrupt instruction with the original the\r\ninstruction cache was either not flushed or not flushed with sufficient length.\r\nThis possibly leads to an increased frequency of the above races.\r\n\r\nNo impact for non-gc stress scenarios.\r\n\r\nAddresses the spurious GC stress failures seen in #17027 and #17610."},"comment":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/comments/382456561","html_url":"https://github.com/dotnet/coreclr/pull/17330#issuecomment-382456561","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/17330","id":382456561,"user":{"login":"sdmaclea","id":25674984,"avatar_url":"https://avatars3.githubusercontent.com/u/25674984?v=4","gravatar_id":"","url":"https://api.github.com/users/sdmaclea","html_url":"https://github.com/sdmaclea","followers_url":"https://api.github.com/users/sdmaclea/followers","following_url":"https://api.github.com/users/sdmaclea/following{/other_user}","gists_url":"https://api.github.com/users/sdmaclea/gists{/gist_id}","starred_url":"https://api.github.com/users/sdmaclea/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdmaclea/subscriptions","organizations_url":"https://api.github.com/users/sdmaclea/orgs","repos_url":"https://api.github.com/users/sdmaclea/repos","events_url":"https://api.github.com/users/sdmaclea/events{/privacy}","received_events_url":"https://api.github.com/users/sdmaclea/received_events","type":"User","site_admin":false},"created_at":"2018-04-18T16:57:28Z","updated_at":"2018-04-18T16:57:28Z","author_association":"MEMBER","body":"> and an instance of #17598 -- in a (x64 gcstress 0xC,)\r\n\r\nI thought this was one of the race types you were trying to fix.\r\n\r\n"}}
{"actor":{"display_login":"sdmaclea"}}