[NO MERGE] SHM_NAME_MAX - #64752

Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom
adamsitnik:issue63240Again
Closed

[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom
adamsitnik:issue63240Again

Conversation

@adamsitnik

@adamsitnikadamsitnik commented Feb 3, 2022

Copy link
Copy Markdown
Member

I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number

Related to #63240

@adamsitnikadamsitnik added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 3, 2022
@ghostghost added the area-System.IO label Feb 3, 2022
@ghostghost assigned adamsitnikFeb 3, 2022
@ghost

ghost commented Feb 3, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number

Related to #63240

Author:adamsitnik
Assignees:-
Labels:

* NO MERGE *, area-System.IO

Milestone:-

@adamsitnik

adamsitnik commented Feb 3, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-staging

@azure-pipelines

azure-pipelinesBot commented Feb 3, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@am11

am11 commented Feb 3, 2022

Copy link
Copy Markdown
Member

It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):

--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif

@adamsitnik

adamsitnik commented Feb 3, 2022

Copy link
Copy Markdown
MemberAuthor

@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?

@adamsitnik

adamsitnik commented Feb 4, 2022

Copy link
Copy Markdown
MemberAuthor

@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?

@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)

BTW is it expected for the staging CI leg to be green despite having test failures?

image

@ericstj

ericstj commented Feb 4, 2022

Copy link
Copy Markdown
Member

BTW is it expected for the staging CI leg to be green despite having test failures?

Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging

how can I force our CI to run the tests on arm64 macOS machines

I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.

@ericstj

ericstj commented Feb 4, 2022

Copy link
Copy Markdown
Member

/azp list

@azure-pipelines

azure-pipelinesBot commented Feb 4, 2022

Copy link
Copy Markdown
CI/CD Pipelines for this repository:

@ericstj

ericstj commented Feb 4, 2022

Copy link
Copy Markdown
Member

/azp run runtime-libraries-coreclr outerloop-osx

@azure-pipelines

azure-pipelinesBot commented Feb 4, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ericstj

ericstj commented Feb 4, 2022•
edited
Loading

Copy link
Copy Markdown
Member

@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results

@adamsitnik

adamsitnik commented Feb 7, 2022

Copy link
Copy Markdown
MemberAuthor

@ericstj thank you!

btw the tests have passed:

image

I was also not able to repro the issue using @jozkee machine.

I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.

@adamsitnik

adamsitnik commented Feb 7, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-libraries-coreclr outerloop-osx

@azure-pipelines

azure-pipelinesBot commented Feb 7, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ericstj

ericstj commented Feb 7, 2022

Copy link
Copy Markdown
Member

@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.
Opened #64929 to add this to the extra-platforms pipeline.

@vcsjones

vcsjones commented Feb 8, 2022

Copy link
Copy Markdown
Member

@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?

akoeplinger
akoeplinger reviewed Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
}
else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
{
--maxNameLength;

@akoeplingerakoeplingerFeb 21, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?

@adamsitnik

adamsitnik commented Apr 26, 2022

Copy link
Copy Markdown
MemberAuthor

Thank you all for your help, I've opened #68541 with hopefully the proper fix.

@adamsitnik
adamsitnik deleted the issue63240Again branch April 26, 2022 10:02
@ghostghost locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
[NO MERGE] SHM_NAME_MAX - #64752
Closed
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Closed
[NO MERGE] SHM_NAME_MAX#64752
adamsitnik wants to merge 1 commit into
dotnet:mainfrom 
adamsitnik:issue63240Again
Conversation
@adamsitnik
@adamsitnikadamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
@adamsitnikadamsitnik
added
the 
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Feb 3, 2022
@ghostghost
added
the 
area-System.IO

label
Feb 3, 2022
@ghostghost
assigned adamsitnikFeb 3, 2022
@ghost
ghost
commented
Feb 3, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io

See info in area-owners.md if you want to be subscribed.
Issue Details
I don't have an ARM64 macOS machine (yet), so I need to use a very dirty way of figuring out the magic number
Related to #63240
Author:adamsitnik
Assignees:-
Labels:
* NO MERGE *, area-System.IO
Milestone:-
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-staging
@azure-pipelines
azure-pipelinesBot
commented
Feb 3, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@am11
am11
commented
Feb 3, 2022

Copy link


Copy Markdown

Member
It is a bit strange to truncate the name in retry attempt on managed side, when we know the (documented) limit in native implementation. Perhaps we need to just replace the asserts with NULL, something like (with or without condition):
--- src/native/libs/System.Native/pal_io.c+++ src/native/libs/System.Native/pal_io.c
#if defined(SHM_NAME_MAX) // macOS - assert(strlen(name) <= SHM_NAME_MAX);+ if (strlen(name) > SHM_NAME_MAX) name[SHM_NAME_MAX] = '\0';
#elif defined(PATH_MAX) // other Unixes - assert(strlen(name) <= PATH_MAX); + if (strlen(name) > PATH_MAX) name[PATH_MAX] = '\0';
#endif 
@adamsitnik
adamsitnik
commented
Feb 3, 2022

Copy link


Copy Markdown

MemberAuthor
@dotnet/area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@adamsitnik
adamsitnik
commented
Feb 4, 2022

Copy link


Copy Markdown

MemberAuthor
@area-infrastructure-libraries how can I force our CI to run the tests on arm64 macOS machines?
@ericstj@safern ping (I need to fix this bug as the backport is incomplete but I can't repro it using the CI or @jozkee M1)
BTW is it expected for the staging CI leg to be green despite having test failures?
image
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
BTW is it expected for the staging CI leg to be green despite having test failures?
Yes, you can see that here https://github.com/dotnet/runtime/blob/4e48641996cfc890163a49ef13bb2121de58d31d/docs/pr-builds.md#runtime-staging
how can I force our CI to run the tests on arm64 macOS machines
I think outerloop and runtime-extra-platforms contain OSX_arm64 that but that might trigger a lot more than what you need, and I believe other resources used by those pipelines are scarce right now. I've pinged the infrastructure channel with this request.
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp list
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


CI/CD Pipelines for this repository:
@ericstj
ericstj
commented
Feb 4, 2022

Copy link


Copy Markdown

Member
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 4, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 4, 2022•

edited

Loading

Copy link


Copy Markdown

Member
@adamsitnik -- this should do it. https://dev.azure.com/dnceng/public/_build/results?buildId=1592720&view=results
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
@ericstj thank you!
btw the tests have passed:
image
I was also not able to repro the issue using @jozkee machine.
I'll try to re-run the CI leg and if it does not fail ask other folks for running it on their macOS arm64 hardware.
@adamsitnik
adamsitnik
commented
Feb 7, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-libraries-coreclr outerloop-osx
@azure-pipelines
azure-pipelinesBot
commented
Feb 7, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@ericstj
ericstj
commented
Feb 7, 2022

Copy link


Copy Markdown

Member
@safern let me know that outerloop isn't a superset, so it won't run innerloop tests ☹️ Not sure how to explicitly trigger InnerLoop on M1. I think you can do it manually in AzDo.

Opened #64929 to add this to the extra-platforms pipeline.
@vcsjones
vcsjones
commented
Feb 8, 2022

Copy link


Copy Markdown

Member
@adamsitnik I ran this PR on my M1 and nothing in System.IO.MemoryMappedFiles/tests failed (828 passed), and I doubled checked I was not running on Rosetta. Is there anything I can do to help you with this on my M1?
akoeplinger
akoeplinger
reviewed
Feb 21, 2022
View reviewed changes
Comment thread...braries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs
 }
 else if (errorInfo.Error == Interop.Error.ENAMETOOLONG)
 {
 --maxNameLength;
@akoeplingerakoeplingerFeb 21, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

should we try to protect against maxNameLength being 0 or negative?
@adamsitnik
adamsitnik
commented
Apr 26, 2022

Copy link


Copy Markdown

MemberAuthor
Thank you all for your help, I've opened #68541 with hopefully the proper fix.
@adamsitnik
adamsitnik
deleted the

issue63240Again

branch
April 26, 2022 10:02
@ghostghost
locked as resolved and limited conversation to collaborators
May 26, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@akoeplingerakoeplingerakoeplinger left review comments

Assignees

@adamsitnikadamsitnik

Labels

area-System.IONO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@adamsitnik@am11@ericstj@vcsjones@akoeplinger