Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COLI-1216: Combinator: File is being used by another process #23

Closed
wants to merge 4 commits into from
Closed

COLI-1216: Combinator: File is being used by another process #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9cbdaa
Fixing exception thrown caused by multiple threads attempting to acce…
aracsibalint Oct 4, 2023
0c7cdcb
Minor changes.
aracsibalint Oct 4, 2023
34d2386
Using simple semaphore (Mutex) to resolve concurrency issue instead o…
aracsibalint Dec 20, 2023
0887b18
Remove and sort usings.
aracsibalint Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions CombinedResourceManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
using System;
using System.Collections.Generic;
using System.Text.RegularExpressions;
using Autofac.Features.Metadata;
using Orchard.Caching;
using Autofac.Features.Metadata;
using Orchard.ContentManagement; // For generic ContentManager methods
using Orchard.DisplayManagement.Descriptors;
using Orchard.DisplayManagement.Descriptors.ResourceBindingStrategy;
Expand All @@ -13,12 +9,14 @@
using Orchard.Settings;
using Orchard.Themes;
using Orchard.UI.Resources;
using Piedone.Combinator.EventHandlers;
using Piedone.Combinator.Extensions;
using Piedone.Combinator.Models;
using Piedone.Combinator.Services;
using System.Linq;
using Piedone.HelpfulLibraries.Utilities;
using System;
using System.Collections.Generic;
using System.Text.RegularExpressions;
using System.Threading;

namespace Piedone.Combinator
{
Expand Down Expand Up @@ -105,9 +103,21 @@ public override IList<ResourceRequiredContext> BuildRequiredResources(string str

IList<ResourceRequiredContext> result;

if (resourceType == ResourceType.Style) result = _combinatorService.CombineStylesheets(resources, settings);
else if (resourceType == ResourceType.JavaScript) result = _combinatorService.CombineScripts(resources, settings);
else return base.BuildRequiredResources(stringResourceType);
using (var mutex = new Mutex(initiallyOwned: false, "Global\\CombinatorFileAccess"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally, this is the same as a simple lock. However, a Mutex is a heavy way of synchronization across processed, what you don't need. This can either be a simple semaphore as I mentioned (with any number of readers and a single writer) or you can do a slightly more sophisticated way with a named locking approach, so the locks are different per resource bundle.

Please decide with a team whether essentially making bundle writing serially executed (across the whole app) is acceptable, or you need a named lock.

{
mutex.WaitOne();

try
{
if (resourceType == ResourceType.Style) result = _combinatorService.CombineStylesheets(resources, settings);
else if (resourceType == ResourceType.JavaScript) result = _combinatorService.CombineScripts(resources, settings);
else return base.BuildRequiredResources(stringResourceType);
}
finally
{
mutex.ReleaseMutex();
}
}

RemoveOriginalResourceShapes(result, resourceType);

Expand Down
71 changes: 53 additions & 18 deletions Services/CacheFileService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using Autofac;
using Orchard;
using Orchard.Caching;
using Orchard.Data;
using Orchard.Environment;
using Orchard.Environment.Configuration;
using Orchard.Environment.Extensions;
using Orchard.Exceptions;
using Orchard.FileSystems.Media;
using Orchard.Mvc;
using Orchard.Services;
using Piedone.Combinator.EventHandlers;
using Piedone.Combinator.Extensions;
using Piedone.Combinator.Models;
using Autofac;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Web.Mvc;

namespace Piedone.Combinator.Services
Expand Down Expand Up @@ -111,11 +109,23 @@ public void Save(string fingerprint, CombinatorResource resource, ICombinatorSet

if (_storageProvider.FileExists(path)) _storageProvider.DeleteFile(path);

using (var stream = _storageProvider.CreateFile(path).OpenWrite())
using (var mutex = new Mutex(initiallyOwned: false, "Global\\CombinatorFileAccess"))
{
var bytes = Encoding.UTF8.GetBytes(resource.Content);
stream.Write(bytes, 0, bytes.Length);
}
mutex.WaitOne();

try
{
using (var stream = _storageProvider.CreateFile(path).OpenWrite())
{
var bytes = Encoding.UTF8.GetBytes(resource.Content);
stream.Write(bytes, 0, bytes.Length);
}
}
finally
{
mutex.ReleaseMutex();
}
}

if (!resource.IsRemoteStorageResource)
{
Expand All @@ -134,11 +144,23 @@ public void Save(string fingerprint, CombinatorResource resource, ICombinatorSet
var relativeUrlsBaseUri = settings.ResourceBaseUri != null ? settings.ResourceBaseUri : new Uri(_urlHelper.RequestContext.HttpContext.Request.Url, _urlHelper.Content("~/"));
ResourceProcessingService.RegexConvertRelativeUrlsToAbsolute(testResource, relativeUrlsBaseUri);

using (var stream = _storageProvider.CreateFile(path).OpenWrite())
using (var mutex = new Mutex(initiallyOwned: false, "Global\\CombinatorFileAccess"))
{
var bytes = Encoding.UTF8.GetBytes(testResource.Content);
stream.Write(bytes, 0, bytes.Length);
}
mutex.WaitOne();

try
{
using (var stream = _storageProvider.CreateFile(path).OpenWrite())
{
var bytes = Encoding.UTF8.GetBytes(testResource.Content);
stream.Write(bytes, 0, bytes.Length);
}
}
finally
{
mutex.ReleaseMutex();
}
}

resource.IsRemoteStorageResource = true;
fileRecord.Settings = _combinatorResourceManager.SerializeResourceSettings(resource);
Expand Down Expand Up @@ -238,9 +260,22 @@ public void WriteSpriteStream(string fileName, SpriteStreamWriter streamWriter)
if (_storageProvider.FileExists(path)) _storageProvider.DeleteFile(path);
var spriteFile = _storageProvider.CreateFile(path);
var publicUrl = _storageProvider.GetPublicUrl(path);
using (var stream = spriteFile.OpenWrite())

using (var mutex = new Mutex(initiallyOwned: false, "Global\\CombinatorFileAccess"))
{
streamWriter(stream, publicUrl);
mutex.WaitOne();

try
{
using (var stream = spriteFile.OpenWrite())
{
streamWriter(stream, publicUrl);
}
}
finally
{
mutex.ReleaseMutex();
}
}
}

Expand Down