Race condition in GetOldestActiveTransactionId()

While hacking on the CSN patch, I spotted a race condition between 
GetOldestActiveTransactionId() and GetNewTransactionId(). 
GetOldestActiveTransactionId() calculates the oldest XID that's still 
running, by doing:

1. Read nextXid, without a lock. This is the upper bound, if there are 
no active XIDs in the proc array.
2. Loop through the proc array, making note of the oldest XID.

While GetNewTransactionId() does:

1. Read and Increment nextXid
2. Set MyPgXact->xid.

It seems possible that if you call GetNewTransactionId() concurrently 
with GetOldestActiveTransactionId(), GetOldestActiveTransactionId() sees 
the new nextXid value that the concurrent GetNewTransactionId() set, but 
doesn't see the old XID in the proc array. It will return a value that 
doesn't cover the old XID, i.e. it won't consider the just-assigned XID 
as in-progress.

Am I missing something? Commit c6d76d7c added a comment to 
GetOldestActiveTransactionId() explaining why it's not necessary to 
acquire XidGenLock there, but I think it missed the above race condition.

GetOldestActiveTransactionId() is not performance-critical, it's only 
called when performing a checkpoint, so I think we should just bite the 
bullet and grab the lock. Per attached patch.

- Heikki