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.

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

atomic requires struct field to be 64-bit aligned #23

Merged
merged 1 commit into from Nov 26, 2015

Conversation

Projects
None yet
4 participants
@tpng
Contributor

tpng commented Nov 26, 2015

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Nov 26, 2015

CLA assistant check
All committers have accepted the CLA.

CLA assistant check
All committers have accepted the CLA.

@mbertschler

This comment has been minimized.

Show comment
Hide comment
@mbertschler

mbertschler Nov 26, 2015

Collaborator

Interesting issue, thank you for the fix.

Collaborator

mbertschler commented Nov 26, 2015

Interesting issue, thank you for the fix.

mbertschler added a commit that referenced this pull request Nov 26, 2015

Merge pull request #23 from tpng/patch-1
atomic requires struct field to be 64-bit aligned

@mbertschler mbertschler merged commit 12fcb8c into spf13:master Nov 26, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@tpng tpng deleted the tpng:patch-1 branch Nov 26, 2015

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Nov 27, 2015

Contributor

@tpng, I'm impressed. How in the world did you come up with that? What lead you to this solution?

Contributor

moorereason commented Nov 27, 2015

@tpng, I'm impressed. How in the world did you come up with that? What lead you to this solution?

@tpng

This comment has been minimized.

Show comment
Hide comment
@tpng

tpng Nov 27, 2015

Contributor

@moorereason I first encountered this issue with the new v0.15 hugo using the server command and found that only the 386 version has this issue.

Then I went adding log.Print in the source code to find where hugo hang and it ends up in (*InMemoryFile).Open().

After that I try to create a repro to see how atomic failed in this case, which crash with a nil pointer error on the call to atomic.StoreInt64(&f.readDirCount, 0).

Finally a google search of golang atomic 386 amd64 ended up with a result that stated that struct field has to be 64-bit aligned and thus the fix.

Contributor

tpng commented Nov 27, 2015

@moorereason I first encountered this issue with the new v0.15 hugo using the server command and found that only the 386 version has this issue.

Then I went adding log.Print in the source code to find where hugo hang and it ends up in (*InMemoryFile).Open().

After that I try to create a repro to see how atomic failed in this case, which crash with a nil pointer error on the call to atomic.StoreInt64(&f.readDirCount, 0).

Finally a google search of golang atomic 386 amd64 ended up with a result that stated that struct field has to be 64-bit aligned and thus the fix.

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Nov 27, 2015

Contributor

That's the first time I've seen a "Bugs" section in the Go stdlib docs. The docs also say this affects non-Linux ARM platforms.

I'm not very familiar with AppVeyor, but it seems like afero could use AppVeyor to automate testing on the Windows x86 platform to catch this sneaky bug.

Contributor

moorereason commented Nov 27, 2015

That's the first time I've seen a "Bugs" section in the Go stdlib docs. The docs also say this affects non-Linux ARM platforms.

I'm not very familiar with AppVeyor, but it seems like afero could use AppVeyor to automate testing on the Windows x86 platform to catch this sneaky bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment