Skip to content

Commit

Permalink
[JENKINS-56446] Avoid FileChannel completely in favor of tracking the…
Browse files Browse the repository at this point in the history
… position manually
  • Loading branch information
dwnusbaum committed Jul 18, 2023
1 parent 7191b33 commit e611d9c
Showing 1 changed file with 19 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.commons.io.input.NullReader;
import org.apache.commons.io.output.CountingOutputStream;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.kohsuke.accmod.Restricted;
Expand All @@ -58,6 +59,9 @@
* Simple implementation of log storage in a single file that maintains a side file with an index indicating where node transitions occur.
* Each line in the index file is a byte offset, optionally followed by a space and then a node ID.
*/
/* Note: Avoid FileChannel methods in this class, as they close the channel and its parent stream if the thread is
interrupted, which is problematic given that we do not control the threads which write to the log file.
*/
@Restricted(Beta.class)
public final class FileLogStorage implements LogStorage {

Expand All @@ -73,6 +77,8 @@ public static synchronized LogStorage forFile(File log) {
private final File index;
@SuppressFBWarnings(value = "IS2_INCONSISTENT_SYNC", justification = "actually it is always accessed within the monitor")
private FileOutputStream os;
private long osStartPosition;
private CountingOutputStream cos;
@SuppressFBWarnings(value = "IS2_INCONSISTENT_SYNC", justification = "we only care about synchronizing writes")
private OutputStream bos;
private Writer indexOs;
Expand All @@ -86,7 +92,9 @@ private FileLogStorage(File log) {
private synchronized void open() throws IOException {
if (os == null) {
os = new FileOutputStream(log, true);
bos = new GCFlushedOutputStream(new DelayBufferedOutputStream(os));
osStartPosition = log.length();
cos = new CountingOutputStream(os);
bos = new GCFlushedOutputStream(new DelayBufferedOutputStream(cos));
if (index.isFile()) {
try (BufferedReader r = Files.newBufferedReader(index.toPath(), StandardCharsets.UTF_8)) {
// TODO would be faster to scan the file backwards for the penultimate \n, then convert the byte sequence from there to EOF to UTF-8 and set lastId accordingly
Expand Down Expand Up @@ -126,28 +134,17 @@ private void checkId(String id) throws IOException {
assert Thread.holdsLock(this);
if (!Objects.equals(id, lastId)) {
bos.flush();
// FileChannel methods such as position() close the channel (and the stream it was derived from) if the
// current thread is interrupted. We must not allow interruption on some arbitrary thread attempting to
// write to the log to close the stream irrevocably, so we clear the interrupt status before using the
// channel and then restore the interrupt status afterwards.
boolean interrupted = Thread.interrupted();
try {
long pos = os.getChannel().position();
if (id == null) {
indexOs.write(pos + "\n");
} else {
indexOs.write(pos + " " + id + "\n");
}
// Could call FileChannel.force(true) like hudson.util.FileChannelWriter does for AtomicFileWriter,
// though making index-log writes slower is likely a poor tradeoff for slightly more reliable log display,
// since logs are often never read and this is transient data rather than configuration or valuable state.
indexOs.flush();
lastId = id;
} finally {
if (interrupted) {
Thread.currentThread().interrupt();
}
long pos = osStartPosition + cos.getByteCount();
if (id == null) {
indexOs.write(pos + "\n");
} else {
indexOs.write(pos + " " + id + "\n");
}
// Could call FileChannel.force(true) like hudson.util.FileChannelWriter does for AtomicFileWriter,
// though making index-log writes slower is likely a poor tradeoff for slightly more reliable log display,
// since logs are often never read and this is transient data rather than configuration or valuable state.
indexOs.flush();
lastId = id;
}
}

Expand Down

0 comments on commit e611d9c

Please sign in to comment.