Windowsでfopenを使ってはいけない!?
Windows環境でsvn+ssh:// からの bzr branch に失敗するという報告があり、調べてみたところ面白いことが判ったので記事にしておきます。
まず、bzr-svnはsubvertpyというlibsvnのバインディングを利用していて、svn+ssh://プロトコルのハンドリングはlibsvnが行っています。 このため、bzr+ssh://ではPython製のSSHクライアントであるparamikoを利用している環境でも、svn+ssh://の場合はlibsvnがssh.exeやSVN_SSH環境変数に設定されたsshクライアントを起動しています。 私はputtyを利用しているので、SVN_SSH=plinkと設定してsvn+sshが利用できる環境を用意して問題が再現することを確認しました。
ログやトレースバックを追ってみたところ、あるファイルを構築する際に一時ファイルに書き込みしていって最後にクローズしてから目的のファイル名にリネームする、というロジックのリネームの部分でPermission Errorが発生していました。
しかし、エラーの原因になっているファイルは、リネーム直前に確実にclose()されています。 なぜPermission Errorが発生しているのかを調べるためにProcess Monitorで該当ファイルに関するシステムコールを追ってみたところ、
- Bazaarがファイルをclose()しているタイミングで、CloseFile()が発行されていない
- なぜかエラー発生後にplink.exeからCloseFile()されている
という事が判りました。
最初は「なんでplink.exeがBazaarの使っているファイルのハンドルを持っているんだ?」と混乱していたのですが、BazaarのIRCでこの事を話してみたところ、 "<jelmer>naoki: It's inheriting handles from the parent process perhaps?" と言われ、Windowsのプロセスとファイルハンドルについて調べてみました。
まず、WindowsのCreateProcess()システムコールは、bInheritHandlesという引数を持っていて、この引数がTRUEの場合ハンドルを子プロセスに引き継ぎます。 引き継がないとパイプが利用できないので、libsvnはbInheritHandles=TRUEでsshクライアントを立ち上げます。
CreateProcess()側でパイプ以外のハンドルを引き継がないような事ができないのですが、 CreateFile()のlpSecurityAttributes引数にSECURITY_ATTRIBUTES構造体を渡すことができ、この中にbInheritHandleというフラグがあります。 このフラグをFALSEにすると、CreateFile()で作ったファイルハンドルは子プロセスに引き継がれないようです。
Pythonの中からCreateFile() API を直接利用するのはちょっと面倒なので、MSVCRTのopen()やfopen()でもできないか調べてみたところ、
- fcntl.h 内で定義されている O_NOINHERIT フラグを open() に渡す
- fopen() の mode として、 "rbN" のように後ろに N をつける
という方法でファイルハンドルを引き継がないようにファイルを開くことが出来ることが判りました。
ここまで判ったところで、Bazaarの修正に取りかかりました。問題になっているファイルはビルトインのopen()関数を使っていて、 この関数のmode引数はそのままfopen()のmode引数になるので、 "wb" となっているところを "wbN" と書き換えるだけで良いかなと思ったのですが、 次のような問題がありました。
- "N" は現在のglibcでは無視されているけれども、将来何かに利用されるかも知れないし、他のlibcで利用されているかも知れない。 なのでWindowsでのみ"N"を使うように修正しないといけない。
- しかも、"N"が有効なのはVC++2005以降のMSVCRTで、それより前のMSVCRTでは無視される。Windows版のPython2.4やPython2.5では使えない。
なので、C言語のopen()関数に相当するPythonのos.open()関数と、os.O_NOINHERITフラグを利用してビルトインのopen()の代わりになる関数を作成し、ファイルをクローズした後にそのファイルを削除やリネームする場所でその関数を使うようにしました。
参考に、今回の修正のうち、open()の代替になっているopen_file()関数の定義部分だけ掲載しておきます。 実際の修正はこの修正のマージリクエスト で見ることが出来ます。また、Python標準ライブラリのtempfileモジュールもos.O_NOINHERITを利用して、一時ファイルを利用した後にファイルを削除できるようにしているので、そちらも参考にして下さい。
O_NOINHERIT = getattr(os, 'O_NOINHERIT', 0) if sys.platform == 'win32': def open_file(filename, mode='r', bufsize=-1): """This function works like builtin ``open``. But use O_NOINHERIT flag so file handle is not inherited to child process. So deleting or renaming closed file that opened with this function is not blocked by child process. """ writing = 'w' in mode appending = 'a' in mode updating = '+' in mode binary = 'b' in mode flags = O_NOINHERIT # see http://msdn.microsoft.com/en-us/library/yeby3zcb%28VS.71%29.aspx # for flags for each modes. if binary: flags |= O_BINARY else: flags |= O_TEXT if writing: if updating: flags |= os.O_RDWR else: flags |= os.O_WRONLY flags |= os.O_CREAT | os.O_TRUNC elif appending: if updating: flags |= os.O_RDWR else: flags |= os.O_WRONLY flags |= os.O_CREAT | os.O_APPEND else: #reading if updating: flags |= os.O_RDWR else: flags |= os.O_RDONLY return os.fdopen(os.open(filename, flags), mode, bufsize) else: open_file = open
この修正が問題なく取り込まれれば、Bazaar 2.1.1からは svn+ssh:// からの bzr branch ができるようになります。