Weird C # error replacing text string

Question

We have a registration type system that sends a confirmation email by completion. Within a few minutes, the system had about 3,000 registrations, and we noticed an error. If user A registers several ms after registration by user B, user A will receive information about user B by e-mail. We managed to fix this problem, and I narrowed it down to this piece of code, which receives the email template from the cache and simply replaces the line on the place holder.

private string ProcessEmailBody(MyRegistrationModel registration) { var content = CacheHelper.GetContent("REGISTRATIONEMAIL"); if (content != null) { content.Text = context.Text.Replace("@@ FULL_NAME@ @", registration.FullName); return content.Text; } else return null; } 

The CacheHelper.GetContent() method is static, and I fixed this "error" by doing the following:

 private string ProcessEmailBody(MyRegistrationModel registration) { var content = CacheHelper.GetContent("REGISTRATIONEMAIL"); if (content != null) { string body = content.Text; body = body.Replace("@@ FULL_NAME@ @", registration.FullName); return body; } else return null; } 

And I can’t understand all my life why this solved the problem. Can anyone shed some light on this?

EDIT : here is my GetContent () method (I know the signatures are different from the previous ones, I was brief)

 public static Content GetContent(string key, int partnerSiteId, int? version, IContentRepository contentRepository, out string cacheKey) { cacheKey = string.Format("{0}_{1}_{2}", key, partnerSiteId, version); var content = CacheManager.Get(cacheKey, () => contentRepository.GetContent(key, partnerSiteId, version), WebConfig.GetCacheDuration(CacheProfile.Short)); return content; } private static DataCache _Cache = null; // DataCache is from AppFabric (Microsoft.ApplicationServer.Caching) public static T Get<T>(string objectKey, Func<T> reloadItemExpresion, TimeSpan cacheDuration) where T : class { if (_Cache == null) { if (reloadItemExpresion != null) { return reloadItemExpresion.Invoke(); } return null; } object cachedObject = null; try { cachedObject = _Cache.Get(objectKey); } catch (Exception ex) { if (ex is FileNotFoundException) { _Cache.Remove(objectKey); } } if (cachedObject != null) { return cachedObject as T; } if (reloadItemExpresion != null && cacheDuration > TimeSpan.Zero) { T item = reloadItemExpresion.Invoke(); if (item != null) { Insert(item, objectKey, cacheDuration); } return item; } return null; } 

contentRepository.GetContent just goes into the database and returns the actual content back.

Answer 1

The first time you replace the tag "@@ FULL_NAME@ @" in context.Text details of the first user. Once you do this, it will not return to "@@ FULL_NAME@ @" again, so everyone will get this guy info until your cache is reset. You should avoid modifying objects that you get from the cache:

 private string ProcessEmailBody(MyRegistrationModel registration) { var content = CacheHelper.GetContent("REGISTRATIONEMAIL"); return content != null ? content.Replace("@@ FULL_NAME@ @", registration.FullName) : null; } 

Answer 2

The problem is that your content object is a shared object between everyone who accesses it. Using the first approach content.Text = context.Text.Replace() , you are going to mutate the text for everyone who accesses it at the same time.

In your second approach, you do not mutate the text inside a common object, so everyone gets their own text at the same time. To avoid this problem in the future, you should consider making your content.Text property read-only for consumers (allowing you to set text only in the constructor or return only read-only interfaces). Thus, avoid this error even during compilation.

Answer 3

It's hard to say without knowing how your CacheHelper method works or what type of content . But it seems that instead of returning a string, it returns a Content object of some type by reference. That way, if two threads start at the same time, both can use the same content object returned by CacheHelper.

Assuming that CacheHelper is responsible for creating a new content template each time you call it, your source code is a mistake in the sense that every call to Replace changes TEMPLATE, not the line received from it.

I assume this code will also work:

 string body = content.Text.Replace("@@ FULL_NAME@ @", registration.FullName); return body; 

The important bit does not read the contents of the Text into a local variable, without replacing the contents of the Text property, which is apparently split.