236807 2022-02-17 15:26:57 -0800 [DataLog] Disable buffering of output only for file, not stderr 2022-02-25 17:25:31 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified ASSIGNED InRadar P2 Normal --- 1 basuke basuke aakash_jain achristensen basuke benjamin cdumez cmarcelo darin don.olmstead ews-watchlist fujii msaboff webkit-bug-importer ysuzuki oldest_to_newest 1843148 0 basuke 2022-02-17 15:26:57 -0800 Stderr is usually unbuffered. This requires only for file. 1843154 1 452440 basuke 2022-02-17 15:32:56 -0800 Created attachment 452440 PATCH 1843389 2 aakash_jain 2022-02-18 07:18:08 -0800 I cancelled https://ews-build.webkit.org/#/builders/68/builds/8529 to speed up ios-wk2 queue. It already finished running layout-tests and the failures seems to be pre-existing. 1844472 3 452440 ysuzuki 2022-02-21 17:32:58 -0800 Comment on attachment 452440 PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=452440&action=review > Source/WTF/ChangeLog:8 > + `stderr` is usually already unbuffered. Disabling buffering is required only for file. I have two questions :) 1. Is it guaranteed that `stderr` is unbuffered? (e.g. specified in C spec etc.). 2. What is the benefit of doing this only for file? If (1) is not guaranteed, always calling setvbuf sounds safer option to me, so I would like to know the motivation. 1844477 4 basuke 2022-02-21 17:39:37 -0800 (In reply to Yusuke Suzuki from comment #3) > Comment on attachment 452440 [details] > PATCH > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452440&action=review > > > Source/WTF/ChangeLog:8 > > + `stderr` is usually already unbuffered. Disabling buffering is required only for file. > > I have two questions :) > > 1. Is it guaranteed that `stderr` is unbuffered? (e.g. specified in C spec > etc.). > 2. What is the benefit of doing this only for file? If (1) is not > guaranteed, always calling setvbuf sounds safer option to me, so I would > like to know the motivation. 1. I use the word "usually" casually without doing a wide research, but as far as I do quick googling, Linux, FreeBSD do disabling buffering by default. If not so, there might be the reason not to do that. stderr is something provided from the system so that it is better to use it as is. 2. Above is the motivation. Not changing the system's expectation is the benefit. 1844488 5 basuke 2022-02-21 18:07:21 -0800 I'll do this by wrapping with PLATFORM macro. 1844787 6 452440 don.olmstead 2022-02-22 10:43:33 -0800 Comment on attachment 452440 PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=452440&action=review >>> Source/WTF/ChangeLog:8 >>> + `stderr` is usually already unbuffered. Disabling buffering is required only for file. >> >> I have two questions :) >> >> 1. Is it guaranteed that `stderr` is unbuffered? (e.g. specified in C spec etc.). >> 2. What is the benefit of doing this only for file? If (1) is not guaranteed, always calling setvbuf sounds safer option to me, so I would like to know the motivation. > > 1. I use the word "usually" casually without doing a wide research, but as far as I do quick googling, Linux, FreeBSD do disabling buffering by default. If not so, there might be the reason not to do that. stderr is something provided from the system so that it is better to use it as is. > > 2. Above is the motivation. Not changing the system's expectation is the benefit. The C standard says in 7.21.3 Files (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2310.pdf) At program startup, three text streams are predefined and need not be opened explicitly — standard input (for reading conventional input), standard output (for writing conventional output), and standard error (for writing diagnostic output). As initially opened, the standard error stream is not fully buffered; the standard input and standard output streams are fully buffered if and only if the stream can be determined not to refer to an interactive device. You should definitely put more information in the CHANGELOG here rather than just a "usually". Looking at documentation does seem to provide a clear case that it is unbuffered on Linux and FreeBSD but I don't know if that's the case with Apple OS's. Windows does seem like it might be buffered https://docs.microsoft.com/en-us/cpp/c-runtime-library/stream-i-o?view=msvc-170 so some kind of platform check is probably needed for this change. 1844797 7 basuke 2022-02-22 11:05:37 -0800 Thanks. Platform dependent macro sounds reasonable for this. 1844822 8 fujii 2022-02-22 12:09:08 -0800 In the spec: > The setvbuf function may be used only after the stream pointed to by stream has been associated > with an open file and before any other operation (other than an unsuccessful call to setvbuf) is > performed on the stream. It seems too late to change stderr here. 1844910 9 452440 darin 2022-02-22 15:37:15 -0800 Comment on attachment 452440 PATCH I think the patch is correct just like this and does not require platform conditionals. I think it’s clear now that it’s an error to call setvbuf on stderr this late in the execution of a program, based on documentation of the cross platform standards that both setvbuf and stderr are part of. There is a highly-unlikely possibility that doing this to stderr is valuable on Apple platforms; someone should determine quickly, by testing, that this is not the case. Then we can land this. 1845365 10 webkit-bug-importer 2022-02-23 15:39:42 -0800 <rdar://problem/89382520> 1846249 11 basuke 2022-02-25 17:25:31 -0800 > I think it’s clear now that it’s an error to call setvbuf on stderr this > late in the execution of a program, based on documentation of the cross > platform standards that both setvbuf and stderr are part of. There is a > highly-unlikely possibility that doing this to stderr is valuable on Apple > platforms; someone should determine quickly, by testing, that this is not > the case. Then we can land this. Thanks Darin. Can somebody in Apple do this test? I want to add description about the effect to Apple platform. 452440 2022-02-17 15:32:56 -0800 2022-02-23 05:25:15 -0800 PATCH patch.diff text/plain 1690 basuke ZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nIGIvU291cmNlL1dURi9DaGFuZ2VMb2cK aW5kZXggM2UyYzgwNTZmMDJhLi4yOTIxOTZmN2RlNzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYv Q2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIw MjItMDItMTcgIEJhc3VrZSBTdXp1a2kgIDxiYXN1a2Uuc3V6dWtpQHNvbnkuY29tPgorCisgICAg ICAgIFtEYXRhTG9nXSBEaXNhYmxlIGJ1ZmZlcmluZyBvZiBvdXRwdXQgb25seSBmb3IgZmlsZSwg bm90IHN0ZGVycgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MjM2ODA3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgYHN0ZGVycmAgaXMgdXN1YWxseSBhbHJlYWR5IHVuYnVmZmVyZWQuIERpc2FibGluZyBidWZm ZXJpbmcgaXMgcmVxdWlyZWQgb25seSBmb3IgZmlsZS4KKworICAgICAgICAqIHd0Zi9EYXRhTG9n LmNwcDoKKyAgICAgICAgKFdURjo6c2V0RGF0YUZpbGUpOgorCiAyMDIyLTAyLTE0ICBHdXN0YXZv IE5vcm9uaGEgU2lsdmEgIDxndXN0YXZvQG5vcm9uaGEuZGV2LmJyPgogCiAgICAgICAgIFtMaW51 eC9hYXJjaDY0XSBNb3ZlIHBhZ2Ugc2l6ZSBjZWlsaW5nIHRvIDE2awpkaWZmIC0tZ2l0IGEvU291 cmNlL1dURi93dGYvRGF0YUxvZy5jcHAgYi9Tb3VyY2UvV1RGL3d0Zi9EYXRhTG9nLmNwcAppbmRl eCAxOTE1MzFmMTAxNmQuLjRhYTA1NWU2ZmU1MCAxMDA2NDQKLS0tIGEvU291cmNlL1dURi93dGYv RGF0YUxvZy5jcHAKKysrIGIvU291cmNlL1dURi93dGYvRGF0YUxvZy5jcHAKQEAgLTEzOCw5ICsx MzgsMTEgQEAgdm9pZCBzZXREYXRhRmlsZShjb25zdCBjaGFyKiBwYXRoKQogICAgICAgICB9CiAK ICAgICAgICAgZmlsZSA9IEZpbGVQcmludFN0cmVhbTo6b3BlbihwYXRoVG9PcGVuLCAidyIpLnJl bGVhc2UoKTsKLSAgICAgICAgaWYgKGZpbGUpCisgICAgICAgIGlmIChmaWxlKSB7CiAgICAgICAg ICAgICBXVEZMb2dBbHdheXMoIioqKiBEYXRhTG9nIG91dHB1dCB0byBcIiVzXCIgKioqXG4iLCBw YXRoVG9PcGVuKTsKLSAgICAgICAgZWxzZQorCisgICAgICAgICAgICBzZXR2YnVmKGZpbGUtPmZp bGUoKSwgbnVsbHB0ciwgX0lPTkJGLCAwKTsgLy8gUHJlZmVyIHVuYnVmZmVyZWQgb3V0cHV0LCBz byB0aGF0IHdlIGdldCBhIGZ1bGwgbG9nIHVwb24gY3Jhc2ggb3IgZGVhZGxvY2suCisgICAgICAg IH0gZWxzZQogICAgICAgICAgICAgV1RGTG9nQWx3YXlzKCJXYXJuaW5nOiBDb3VsZCBub3Qgb3Bl biBEYXRhTG9nIGZpbGUgJXMgZm9yIHdyaXRpbmcuXG4iLCBwYXRoVG9PcGVuKTsKICAgICB9CiAK QEAgLTE1MCw4ICsxNTIsNiBAQCB2b2lkIHNldERhdGFGaWxlKGNvbnN0IGNoYXIqIHBhdGgpCiAg ICAgICAgIGZpbGUgPSBuZXcgKHNfZmlsZURhdGEpIEZpbGVQcmludFN0cmVhbShzdGRlcnIsIEZp bGVQcmludFN0cmVhbTo6Qm9ycm93KTsKICAgICB9CiAKLSAgICBzZXR2YnVmKGZpbGUtPmZpbGUo KSwgbnVsbHB0ciwgX0lPTkJGLCAwKTsgLy8gUHJlZmVyIHVuYnVmZmVyZWQgb3V0cHV0LCBzbyB0 aGF0IHdlIGdldCBhIGZ1bGwgbG9nIHVwb24gY3Jhc2ggb3IgZGVhZGxvY2suCi0KICAgICBpZiAo c19maWxlKQogICAgICAgICBzX2ZpbGUtPmZsdXNoKCk7CiANCg==