Skip to content

Conversation

liliankasem
Copy link
Member

Issue describing the changes in this PR

resolves #issue_for_this_pr

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

@@ -84,6 +86,9 @@ public IServiceProvider ConfigureServices(IServiceCollection services)

services.AddWebJobsScriptHost(_builderContext.Configuration);

// Replace the default MemoryMappedFileAccessor with a null implementation
services.Replace(ServiceDescriptor.Singleton<IMemoryMappedFileAccessor, NullMemoryMappedFileAccessor>());
Copy link
Member Author

Choose a reason for hiding this comment

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

@fabiocav draft here; still need to consider all the potential impact of disabling this - let me know what you think

Copy link
Member Author

Choose a reason for hiding this comment

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

But I think we still want to allow users to test this feature right? So maybe disable by default and add a flag to bring this back in. Or we just don't make a change here and fix it on the host side?

public override void Delete(string mapName, MemoryMappedFile mmf)
{
Logger.LogDebug("NullMemoryMappedFileAccessor: Delete called for {MapName} - no action taken", mapName);
mmf?.Dispose();
Copy link
Member

Choose a reason for hiding this comment

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

Should we care about mmf not being null or not already disposed?

/// A null implementation of MemoryMappedFileAccessor that disables shared memory functionality.
/// This can be used for testing or in environments where shared memory is not supported or desired.
/// </summary>
public class NullMemoryMappedFileAccessor : MemoryMappedFileAccessor
Copy link
Member

Choose a reason for hiding this comment

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

Seal it to prevent unintended extension.

Logger.LogDebug("Using NullMemoryMappedFileAccessor - shared memory data transfer is disabled");
}

public override bool TryCreate(string mapName, long size, out MemoryMappedFile mmf)
Copy link
Member

Choose a reason for hiding this comment

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

Adding a comment can help - "/// Always returns false; no shared memory is created. ///"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants