Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
MIME-Version: 1.0
From: Evgeny Kotkov <evgeny.kotkov@visualsvn.com>
Date: Wed, 30 Aug 2017 01:22:24 +0300
Message-ID: <CAP_GPNhSq_ga=C3v1MpDMu414KjBwNGXmXEeDTE5DN6hO-taNQ@mail.gmail.com>
Subject: [PATCH] Don't seek to the end when opening files with APR_FOPEN_APPEND
To: dev@apr.apache.org
Content-Type: multipart/mixed; boundary="001a1145c492b7fa040557ebd97f"
archived-at: Tue, 29 Aug 2017 22:24:19 -0000

--001a1145c492b7fa040557ebd97f
Content-Type: text/plain; charset="UTF-8"

Hi everyone,

Currently, the Windows implementation of APR_FOPEN_APPEND flag performs
a seek to the file's end when opening it.

Apparently, this is a leftover from the very first version of handling file
appends (https://svn.apache.org/r59449) that performed a single seek to
the file's end when opening it and did not support proper atomic appends
with multiple process or threads writing to the same file.

Since then, such atomic appends have been implemented, but the seek
was not removed.  It can cause unexpected behavior when reading from
a file opened with APR_FOPEN_APPEND, assuming no writes happened
to the file.  In this case, as there have been no writes, the file offset
should not be repositioned and reading should start from the beginning of
the file.  However, due to the unwanted seek during open, the actual reading
would instead start from the file's end and cause an unexpected EOF.

The attached patch fixes the issue and adds a test for it.  The log message
is included in the beginning of the patch file.


Thanks,
Evgeny Kotkov

--001a1145c492b7fa040557ebd97f
Content-Type: text/plain; charset="US-ASCII";
	name="dont-seek-when-opening-file-for-append-v1.patch.txt"
Content-Disposition: attachment;
	filename="dont-seek-when-opening-file-for-append-v1.patch.txt"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_j6y5l6aw0

V2luMzI6IERvbid0IHNlZWsgdG8gdGhlIGVuZCB3aGVuIG9wZW5pbmcgZmlsZXMgd2l0aCBBUFJf
Rk9QRU5fQVBQRU5ELg0KDQpBcHBhcmVudGx5LCB0aGlzIGlzIGEgbGVmdG92ZXIgZnJvbSB0aGUg
dmVyeSBmaXJzdCB2ZXJzaW9uIG9mIGhhbmRsaW5nIGZpbGUNCmFwcGVuZHMgKGh0dHBzOi8vc3Zu
LmFwYWNoZS5vcmcvcjU5NDQ5KSB0aGF0IHBlcmZvcm1lZCBhIHNpbmdsZSBzZWVrIHRvDQp0aGUg
ZmlsZSdzIGVuZCB3aGVuIG9wZW5pbmcgaXQgYW5kIGRpZCBub3Qgc3VwcG9ydCBwcm9wZXIgYXRv
bWljIGFwcGVuZHMNCndpdGggbXVsdGlwbGUgcHJvY2VzcyBvciB0aHJlYWRzIHdyaXRpbmcgdG8g
dGhlIHNhbWUgZmlsZS4NCg0KU2luY2UgdGhlbiwgc3VjaCBhdG9taWMgYXBwZW5kcyBoYXZlIGJl
ZW4gaW1wbGVtZW50ZWQsIGJ1dCB0aGUgc2Vlaw0Kd2FzIG5vdCByZW1vdmVkLiAgSXQgY2FuIGNh
dXNlIHVuZXhwZWN0ZWQgYmVoYXZpb3Igd2hlbiByZWFkaW5nIGZyb20NCmEgZmlsZSBvcGVuZWQg
d2l0aCBBUFJfRk9QRU5fQVBQRU5ELCBhc3N1bWluZyBubyB3cml0ZXMgaGFwcGVuZWQNCnRvIHRo
ZSBmaWxlLiAgSW4gdGhpcyBjYXNlLCBhcyB0aGVyZSBoYXZlIGJlZW4gbm8gd3JpdGVzLCB0aGUg
ZmlsZSBvZmZzZXQNCnNob3VsZCBub3QgYmUgcmVwb3NpdGlvbmVkIGFuZCByZWFkaW5nIHNob3Vs
ZCBzdGFydCBmcm9tIHRoZSBiZWdpbm5pbmcgb2YNCnRoZSBmaWxlLiAgSG93ZXZlciwgZHVlIHRv
IHRoZSB1bndhbnRlZCBzZWVrIGR1cmluZyBvcGVuLCB0aGUgYWN0dWFsIHJlYWRpbmcNCndvdWxk
IGluc3RlYWQgc3RhcnQgZnJvbSB0aGUgZmlsZSdzIGVuZCBhbmQgY2F1c2UgYW4gdW5leHBlY3Rl
ZCBFT0YuDQoNCiogZmlsZV9pby93aW4zMi9vcGVuLmMNCiAgKGFwcl9maWxlX29wZW4pOiBEb24n
dCBzZWVrIHRvIHRoZSBmaWxlJ3MgZW5kIHdoZW4gdGhlIGZpbGUgaXMNCiAgIG9wZW5lZCB3aXRo
IEFQUl9GT1BFTl9BUFBFTkQuDQoNCiogdGVzdC90ZXN0ZmlsZS5jDQogICh0ZXN0X2FwcGVuZF9y
ZWFkKTogTmV3IHRlc3QuDQogICh0ZXN0ZmlsZSk6IFJ1biB0aGUgbmV3IHRlc3QuDQoNClBhdGNo
IGJ5OiBFdmdlbnkgS290a292IDxldmdlbnkua290a292IHthdH0gdmlzdWFsc3ZuLmNvbT4NCg0K
SW5kZXg6IGZpbGVfaW8vd2luMzIvb3Blbi5jDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gZmlsZV9pby93aW4z
Mi9vcGVuLmMJKHJldmlzaW9uIDE4MDY2NDUpDQorKysgZmlsZV9pby93aW4zMi9vcGVuLmMJKHdv
cmtpbmcgY29weSkNCkBAIC00NDUsNyArNDQ1LDYgQEAgQVBSX0RFQ0xBUkUoYXByX3N0YXR1c190
KSBhcHJfZmlsZV9vcGVuKGFwcl9maWxlX3QNCiAKICAgICBpZiAoZmxhZyAmIEFQUl9GT1BFTl9B
UFBFTkQpIHsKICAgICAgICAgKCpuZXcpLT5hcHBlbmQgPSAxOwotICAgICAgICBTZXRGaWxlUG9p
bnRlcigoKm5ldyktPmZpbGVoYW5kLCAwLCBOVUxMLCBGSUxFX0VORCk7CiAgICAgfQogICAgIGlm
IChmbGFnICYgQVBSX0ZPUEVOX0JVRkZFUkVEKSB7CiAgICAgICAgICgqbmV3KS0+YnVmZmVyZWQg
PSAxOwpJbmRleDogdGVzdC90ZXN0ZmlsZS5jDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gdGVzdC90ZXN0Zmls
ZS5jCShyZXZpc2lvbiAxODA2NjQ1KQ0KKysrIHRlc3QvdGVzdGZpbGUuYwkod29ya2luZyBjb3B5
KQ0KQEAgLTE3OTMsNiArMTc5Myw1NiBAQCBzdGF0aWMgdm9pZCB0ZXN0X2FwcGVuZF9sb2NrZWQo
YWJ0c19jYXNlICp0Yywgdm9pZA0KICAgICBhcHJfZmlsZV9yZW1vdmUoZm5hbWUsIHApOwogfQog
CitzdGF0aWMgdm9pZCB0ZXN0X2FwcGVuZF9yZWFkKGFidHNfY2FzZSAqdGMsIHZvaWQgKmRhdGEp
Cit7CisgICAgYXByX3N0YXR1c190IHJ2OworICAgIGFwcl9maWxlX3QgKmY7CisgICAgY29uc3Qg
Y2hhciAqZm5hbWUgPSAiZGF0YS90ZXN0YXBwZW5kX3JlYWQuZGF0IjsKKyAgICBhcHJfb2ZmX3Qg
b2Zmc2V0OworICAgIGNoYXIgYnVmWzY0XTsKKworICAgIGFwcl9maWxlX3JlbW92ZShmbmFtZSwg
cCk7CisKKyAgICAvKiBDcmVhdGUgZmlsZSB3aXRoIGNvbnRlbnRzLiAqLworICAgIHJ2ID0gYXBy
X2ZpbGVfb3BlbigmZiwgZm5hbWUsIEFQUl9GT1BFTl9XUklURSB8IEFQUl9GT1BFTl9DUkVBVEUs
CisgICAgICAgICAgICAgICAgICAgICAgIEFQUl9GUFJPVF9PU19ERUZBVUxULCBwKTsKKyAgICBB
UFJfQVNTRVJUX1NVQ0NFU1ModGMsICJjcmVhdGUgZmlsZSIsIHJ2KTsKKworICAgIHJ2ID0gYXBy
X2ZpbGVfcHV0cygiYWJjIiwgZik7CisgICAgQVBSX0FTU0VSVF9TVUNDRVNTKHRjLCAid3JpdGUg
dG8gZmlsZSIsIHJ2KTsKKyAgICBhcHJfZmlsZV9jbG9zZShmKTsKKworICAgIC8qIFJlLW9wZW4g
aXQgd2l0aCBBUFJfRk9QRU5fQVBQRU5ELiAqLworICAgIHJ2ID0gYXByX2ZpbGVfb3BlbigmZiwg
Zm5hbWUsCisgICAgICAgICAgICAgICAgICAgICAgIEFQUl9GT1BFTl9SRUFEIHwgQVBSX0ZPUEVO
X1dSSVRFIHwgQVBSX0ZPUEVOX0FQUEVORCwKKyAgICAgICAgICAgICAgICAgICAgICAgQVBSX0ZQ
Uk9UX09TX0RFRkFVTFQsIHApOworICAgIEFQUl9BU1NFUlRfU1VDQ0VTUyh0YywgIm9wZW4gZmls
ZSIsIHJ2KTsKKworICAgIC8qIFRlc3QgdGhlIGluaXRpYWwgZmlsZSBvZmZzZXQuICBFdmVuIHRo
b3VnaCB3ZSB1c2VkIEFQUl9GT1BFTl9BUFBFTkQsCisgICAgICogdGhlIG9mZnNldCBzaG91bGQg
YmUga2VwdCBpbiB0aGUgYmVnaW5uaW5nIG9mIHRoZSBmaWxlIHVudGlsIHRoZQorICAgICAqIGZp
cnN0IGFwcGVuZC4gIChQcmV2aW91c2x5LCB0aGUgV2luZG93cyBpbXBsZW1lbnRhdGlvbiBwZXJm
b3JtZWQKKyAgICAgKiBhbiBlcnJvbmVvdXMgc2VlayB0byB0aGUgZmlsZSdzIGVuZCByaWdodCBh
ZnRlciBvcGVuaW5nIGl0LikKKyAgICAgKi8KKyAgICBvZmZzZXQgPSAwOworICAgIHJ2ID0gYXBy
X2ZpbGVfc2VlayhmLCBBUFJfQ1VSLCAmb2Zmc2V0KTsKKyAgICBBUFJfQVNTRVJUX1NVQ0NFU1Mo
dGMsICJnZXQgZmlsZSBvZmZzZXQiLCBydik7CisgICAgQUJUU19JTlRfRVFVQUwodGMsIDAsIChp
bnQpb2Zmc2V0KTsKKworICAgIC8qIFRlc3QgcmVhZGluZyBmcm9tIHRoZSBmaWxlLiAqLworICAg
IHJ2ID0gYXByX2ZpbGVfZ2V0cyhidWYsIHNpemVvZihidWYpLCBmKTsKKyAgICBBUFJfQVNTRVJU
X1NVQ0NFU1ModGMsICJyZWFkIGZyb20gZmlsZSIsIHJ2KTsKKyAgICBBQlRTX1NUUl9FUVVBTCh0
YywgImFiYyIsIGJ1Zik7CisKKyAgICAvKiBUZXN0IHRoZSBmaWxlIG9mZnNldCBhZnRlciByZWFk
aW5nLiAqLworICAgIG9mZnNldCA9IDA7CisgICAgcnYgPSBhcHJfZmlsZV9zZWVrKGYsIEFQUl9D
VVIsICZvZmZzZXQpOworICAgIEFQUl9BU1NFUlRfU1VDQ0VTUyh0YywgImdldCBmaWxlIG9mZnNl
dCIsIHJ2KTsKKyAgICBBQlRTX0lOVF9FUVVBTCh0YywgMywgKGludClvZmZzZXQpOworCisgICAg
YXByX2ZpbGVfY2xvc2UoZik7CisgICAgYXByX2ZpbGVfcmVtb3ZlKGZuYW1lLCBwKTsKK30KKwog
YWJ0c19zdWl0ZSAqdGVzdGZpbGUoYWJ0c19zdWl0ZSAqc3VpdGUpCiB7CiAgICAgc3VpdGUgPSBB
RERfU1VJVEUoc3VpdGUpCkBAIC0xODQ4LDYgKzE4OTgsNyBAQCBhYnRzX3N1aXRlICp0ZXN0Zmls
ZShhYnRzX3N1aXRlICpzdWl0ZSkNCiAgICAgYWJ0c19ydW5fdGVzdChzdWl0ZSwgdGVzdF93cml0
ZV9idWZmZXJlZF9zcGFubmluZ19vdmVyX2J1ZnNpemUsIE5VTEwpOwogICAgIGFidHNfcnVuX3Rl
c3Qoc3VpdGUsIHRlc3RfYXRvbWljX2FwcGVuZCwgTlVMTCk7CiAgICAgYWJ0c19ydW5fdGVzdChz
dWl0ZSwgdGVzdF9hcHBlbmRfbG9ja2VkLCBOVUxMKTsKKyAgICBhYnRzX3J1bl90ZXN0KHN1aXRl
LCB0ZXN0X2FwcGVuZF9yZWFkLCBOVUxMKTsKIAogICAgIHJldHVybiBzdWl0ZTsKIH0K
--001a1145c492b7fa040557ebd97f--