Should use THREADCHECK instead of ASSERT_R_THREAD()

[wch]

As of
wch/r-source@3c59cea, R itself has a THREADCHECK macro to make sure that various functions are called from the correct thread.

Commit wch/r-source@5cf1f4b also added the checks that were in ASSERT_R_THREAD().

The assertthread build should be changed to use the built-in checks.

[jeroen]

Cool! Do these macro's prevent the GC from running in a non-main thread?

[wch]

Yes, I believe that if it tries to do GC in a thread, it calls R_suicide(), which I assume kills the R process. :)

The first link above was previously incorrect, but I've fixed it, and if you follow it you can see how it works. Just curious: in your work, have you had problems with GC in other threads?

[jeroen]

I avoid calling the R api in threads because afaik none of the api's are thread-safe due to the random GC runs. I was hoping these changes could help with suspending the GC in threads, but I guess that is not the purpose here :)

[wch]

Fixed in e9899c3.

[wch]

@jeroen I think the problem is even worse than having GC events run in the wrong thread. If you do any R memory management at all in a different thread, there's the possibility of memory corruption due to races. For example, if you look at wch/r-source@5cf1f4b, you can see that protect() and unprotect() manipulate R_PPStack, and similarly, R_PreserveObject() and R_ReleaseObject() manipulate R_PreciousList. In the past I have run into many weird, hard-to-reproduce bugs because of these races.

But I agree about the overall lesson: don't call the R API from a different thread.