Azure-Samples/azure-cache-redis-samples

Incorrect order of replacing old connection with new one

Opened this issue · 0 comments

This issue is for a: (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Mention any other details that might be useful

From line 145 to line 159 there's the following logic:

  1. Close the current connection
  2. Replace _connection with null
  3. Create a new connection
  4. Replace _connection with the new connection

The issue is, that it opens a window for NullReferenceException and ObjectDisposedException exceptions:

  • Any thread holding the current connection reference after 1 has run, but before 2 has run will get an ObjectDisposedException.
  • Any thread using _connection after 2 has run and before 4 has run will get a NullReferenceException.

The correct order should be:

  1. Create a new connection
  2. Swap current connection with the new connection (atomic)
  3. Dispose old connection - this one can still cause problems to any thread still using the connection, not sure what to do with it.