* flockが不徹底 [#wfdb9ebc]
- 元タイトル: flock処理が不完全
-ページ: [[BugTrack2]]
-投稿者: [[Cue]]
-優先順位: 重要
-状態: 提案
-カテゴリー: 本体バグ
-投稿日: 2005-07-30 (土) 19:43:50
-バージョン:
**メッセージ [#jc559974]
影響範囲を調べきれていませんが、get_source等でflock処理がされていません。flockでLOCK_EXを行なってもflockを使っていない処理のファイルアクセスはブロックできないので、file_write側だけでロックしても十分な効果は得られません。
----
-関連:[[BugTrack/164]] -- [[Ratbeta]] &new{2005-07-30 (土) 19:54:06};
--164の場合はflockをサポートしないファイルシステム等でどうやってロックするかの話みたいです。こちらの話はflockをサポートするシステムでも不完全になっているという話です。そのままになってるという事は恐らく全てのバージョンで問題がある気がします…。 -- [[Cue]] &new{2005-07-31 (日) 00:38:11};
-こんにちは :) flock() に関するご指摘は技術的に正確だと思います。ファイルアクセスを行う所では、危険な領域を明確にした上で、読み込みアクセスを行う所なら flock(LOCK_SH) 、書き込む所なら flock(LOCK_EX) を明確に行うべきです。flock()を使うのであれば、全ての部分でそれを行わなければなりません。 -- [[henoheno]] &new{2005-07-31 (日) 11:22:20};
-- PHPのfile()関数は内部でflock(LOCK_SH)などを行いませんので、「他のプロセスが排他ロック(LOCK_EX) を行って､まさに書き込んでいる最中の」中途半端なデータも基本的に読み取ることができてしまいます。 -- [[henoheno]] &new{2005-07-31 (日) 11:24:27};
-- その一方でPukiWikiは基本的に書き込みよりも読み取りの比率が圧倒的に多く、ページの更新に対してはMD5ダイジェストによる衝突検知を行っているため、通常の利用の範囲においては致命的な状況が発生することが今まで少なかったのではないかと思われます。 -- [[henoheno]] &new{2005-07-31 (日) 11:29:27};
-- この問題の根本は、PukiWikiのソースについて今まで統一的な視点による継続的な見直しが行われておらず、技術的な観点がコードごとに発散している所にあると思います。flock()についても有無を含め(効果的な)使い方について精査する必要があると思います。 -- [[henoheno]] &new{2005-07-31 (日) 11:42:45};
--- flock() の有無と使い方に関する私の見解は: [[BugTrack2/83]] (のソース) -- [[henoheno]] &new{2005-07-31 (日) 11:49:04};
-- flock(LOCK_SH) を追加しました。[[cvs:lib/file.php]] (1.33) -- [[henoheno]] &new{2005-07-31 (日) 12:14:46};
-- [[cvs:lib/file.php]] (1.34) lock中にget_source() したい方のために、ロックしないオプションを追加。 -- [[henoheno]] &new{2005-08-02 (火) 00:05:02};
--- 既存のコードを念のため見回りましたが、get_source() をflock()で囲んでいるような状況は見当たりませんでした。 -- [[henoheno]] &new{2005-08-02 (火) 00:17:51};
-とりあえず 1.4.6 向けの修正はここまでとさせていただきます :) 別途あれば少しづつ片付けて行きましょう -- [[henoheno]] &new{2005-08-07 (日) 23:21:22};
- 関連: [[BugTrack2/306]] -- &new{2009-08-13 (木) 14:28:15};
- [[pukiwiki:質問箱3/260]] -- &new{2010-10-24 (日) 12:29:30};
- [[official:質問箱3/260]] -- &new{2010-10-24 (日) 12:29:30};
#comment
** fopen(w) と flock(LOCK_EX) [#qc64c750]
-対応おつかれさまです。172行file_write中のfopen($file, 'w')も変更お願いします。lock前にファイルサイズ0になります -- &new{2005-07-31 (日) 12:57:42};
-かゆい所のお知らせありがとうございます :) 誰かが空の文書を掴みそうで気持ち悪いですね (^^; -- [[henoheno]] &new{2005-08-02 (火) 00:00:38};
-[[cvs:lib/file.php]] (1.35-1.37) こんなところでしょうか。-- 1.36のままではファイルが存在しなかった時にfopenで落ちる (^^; -- [[henoheno]] &new{2005-08-02 (火) 00:05:57};
-1.4.6向けにはここまでとさせていただきます。他にも類似の例があればお知らせください :) -- [[henoheno]] &new{2005-08-07 (日) 23:23:42};
#comment
** その他: flock() を行うラッパーを使う案 [#a861ca24]
-grepするとプラグインも含めてかなりの修正個所がありそうなので、こんな関数をどこかに置いて呼び出す形にしてはどうでしょう？後々BugTrack/164の対応する時にもflockが一ヶ所に集約されていた方が都合が良いかと。 -- [[Cue]] &new{2005-08-02 (火) 22:48:39};
function fopen_w_lock($filename, $mode, $use_include_path = FALSE)
{
$mode = str_replace('b', '', $mode);
$fp = @fopen($filename, 'a+b', $use_include_path);
if($fp !== FALSE){
switch($mode){
case 'r':
flock($fp, LOCK_SH);
rewind($fp);
break;
case 'r+':
flock($fp, LOCK_EX);
rewind($fp);
break;
case 'w':
case 'w+':
flock($fp, LOCK_EX);
ftruncate($fp, 0);
break;
case 'a':
case 'a+':
flock($fp, LOCK_EX);
fseek($fp, SEEK_END);
}
}
return $fp;
}
function fclose_w_lock(&$fp)
{
fflush($fp);
flock($fp, LOCK_UN);
fclose($fp);
}
function file_w_lock($filename, $use_include_path = FALSE)
{
$fp = &fopen_w_lock($filename, 'r', $use_include_path);
if($fp === FALSE)
return FALSE;
$ary = file($filename, $use_include_path);
fclose_w_lock($fp);
return $ary;
}
-興味深いアイデアですね :) しかし「flock()を使っていない => 気にしなくても済む様にすれば良い」と言うのでは事態の歯止めにはなりません。求めるべきは「目的をきちんとわかっているコード」や、「それを書け、また検証できる人々」かと思います。他にも沢山修正点があるとの事ですが、それは fopen() で grep されたのでしょうか? -- [[henoheno]] &new{2005-08-07 (日) 23:20:43};
-PHP >= 4.3.2限定なら stream_wrapper_register が使えるかもしれない -- &new{2005-08-05 (金) 09:09:23};
-- こちらは file() などが内部で stream を呼び出す様になっている件ですよね :) 考え方としては同じと受け止めております -- [[henoheno]] &new{2005-08-07 (日) 23:25:19};
-えっと、検索対象にしたのはfopen,gzopen,fileで一応タグジャンプして周辺コードは見てみました。1.4.6向けはこれで打ち止めとの事なので落ちついたら一度試してみてください。 -- [[Cue]] &new{2005-08-08 (月) 01:28:32};
-- なるほど、gzopenということはbackup、そしてdiffなどのメディアも怪しいということでしょうね :) -- [[henoheno]] &new{2005-08-08 (月) 21:16:02};
--flockは気にしないのなら中途半端に対策するより放置した方が良いのかもしれません。ただ、backup周りは今の1ファイル管理仕様だと一度何かあると全滅になるので、flockもそうですが、世代毎に別ファイルにする等の対策が要るような気はしてます(200世代くらいになるとメモリも解凍時間もきついみたいですし)。話がそれましたが、flockは今対策してある部分を確実にするのが第一歩かと。 -- [[Cue]] &new{2005-08-08 (月) 22:43:24};
---気になるのでflockだけ。処理が特殊なので穴があるかもしれません。 -- [[Cue]] &new{2005-08-21 (日) 20:18:19};
--- backup.php Sat Apr 30 14:21:00 2005
+++ backup.lock.php Sun Aug 21 18:56:52 2005
@@ -65,12 +65,32 @@
$body = PKWK_SPLITTER . ' ' . get_filetime($page) . "\n" . join('', $body);
$body = preg_replace("/\n*$/", "\n", $body);
- $fp = _backup_fopen($page, 'wb')
- or die_message('cannot write file ' . htmlspecialchars($realfilename) .
- '<br />maybe permission is not writable or filename is too long');
- _backup_fputs($fp, $strout);
- _backup_fputs($fp, $body);
- _backup_fclose($fp);
+ $filename = _backup_get_filename($page);
+ do{
+ if(($lock = @fopen($filename, 'r')) !== FALSE){
+ flock($lock, LOCK_EX);
+ clearstatcache();
+ if(_backup_get_filetime($page) != $lastmod)
+ break;
+ }else{
+ $lock = @fopen($filename, 'a+')
+ or die_message('cannot write file ' . htmlspecialchars($filename) .
+ '<br />maybe permission is not writable or filename is too long');
+ flock($lock, LOCK_EX);
+ // $strout = ''; // 処理中に管理者がファイルを消したとしたら?
+ clearstatcache();
+ if(filesize($filename) != 0)
+ break;
+ }
+ $fp = _backup_fopen($page, 'wb')
+ or die_message('cannot write file ' . htmlspecialchars($filename) .
+ '<br />maybe permission is not writable or filename is too long');
+ _backup_fputs($fp, $strout);
+ _backup_fputs($fp, $body);
+ _backup_fclose($fp);
+ }while(FALSE);
+ flock($lock, LOCK_UN);
+ fclose($lock);
}
}
@@ -230,9 +250,15 @@
*/
function _backup_file($page)
{
- return _backup_file_exists($page) ?
- gzfile(_backup_get_filename($page)) :
- array();
+ $filename = _backup_get_filename($page);
+ $data = array();
+ if(($fp = @fopen($filename, 'r')) !== FALSE){
+ flock($fp, LOCK_SH);
+ $data = gzfile($filename);
+ flock($fp, LOCK_UN);
+ fclose($fp);
+ }
+ return $data !== FALSE ? $data : array();
}
}
/////////////////////////////////////////////////
@@ -296,9 +322,15 @@
*/
function _backup_file($page)
{
- return _backup_file_exists($page) ?
- file(_backup_get_filename($page)) :
- array();
+ $filename = _backup_get_filename($page);
+ $data = array();
+ if(($fp = @fopen($filename, 'r')) !== FALSE){
+ flock($fp, LOCK_SH);
+ $data = file($filename);
+ flock($fp, LOCK_UN);
+ fclose($fp);
+ }
+ return $data !== FALSE ? $data : array();
}
}
?>
#comment
![[PukiWiki]](image/pukiwiki-dev.png "[PukiWiki]")
