Skip to content

Conversation

@klynch
Copy link
Contributor

@klynch klynch commented Oct 17, 2011

I got into a state where the file existed but was still locked. Ensuring the unlock guarantees the file will be unlocked even if an exception arises in the block.

Also, Timeout::timeout(20) can be called like Timeout::timeout(20, Gitosis::AccessDenied) to cast timeout exceptions automatically. This will prevent exceptions in yield from being recast to Gitosis specific exceptions.

Also, you might want to consider removing the file after the lock.

@dzaporozhets
Copy link
Contributor

ok i'll check it later. seems ok

@klynch
Copy link
Contributor Author

klynch commented Oct 17, 2011

So backstory on my issue. I was having trouble getting things set up, so I
began running gitlab as root. The file was created 744 or 700, can't
remember. Gitosis.new.configure was failing because it couldn't connect.
When I realized root needed to access ~/.ssh/id_rsa I began running the
process as my user, but it couldn't access the lock file... I couldn't
figure out the bug until I removed the recasting of the exception and
finally removed the file.

Thinking more about it, the ensure that I wrote should probably remove the
file after all... doing this, the semantics of ensure will remove the file
no matter when the ruby process closes (except if a sigkill is issued, I
believe).

On Mon, Oct 17, 2011 at 3:56 AM, Dmitriy Zaporozhets <
reply@reply.github.com>wrote:

ok i'll check it later. seems ok

Reply to this email directly or view it on GitHub:
#31 (comment)

dzaporozhets added a commit that referenced this pull request Oct 18, 2011
Must ensure that the lock is always removed.
@dzaporozhets dzaporozhets merged commit 757ea63 into gitlabhq:1x Oct 18, 2011
Rovanion pushed a commit to Rovanion/gitlabhq that referenced this pull request Nov 15, 2013
tigefa4u referenced this pull request in tigefa4u/gitlabhq Jul 20, 2019
tigefa4u referenced this pull request in tigefa4u/gitlabhq Jul 20, 2019
document all custom repo options

Fixes #31

See merge request !53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants