Skip to content

Commit 720bfce

Browse files
committed
Add debug logging for piped_process failures
1 parent 9c37ad8 commit 720bfce

File tree

4 files changed

+35
-19
lines changed

4 files changed

+35
-19
lines changed

src/solvers/smt2_incremental/smt_solver_process.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ smt_piped_solver_processt::smt_piped_solver_processt(
1313
std::string command_line,
1414
message_handlert &message_handler)
1515
: command_line_description{"\"" + command_line + "\""},
16-
process{split_string(command_line, ' ', false, true)},
16+
process{split_string(command_line, ' ', false, true), message_handler},
1717
log{message_handler}
1818
{
1919
}

src/util/piped_process.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ prepare_windows_command_line(const std::vector<std::string> &commandvec)
124124
}
125125
#endif
126126

127-
piped_processt::piped_processt(const std::vector<std::string> &commandvec)
127+
piped_processt::piped_processt(
128+
const std::vector<std::string> &commandvec,
129+
message_handlert &message_handler)
130+
: log{message_handler}
128131
{
129132
# ifdef _WIN32
130133
// Security attributes for pipe creation
@@ -365,12 +368,17 @@ piped_processt::send_responset piped_processt::send(const std::string &message)
365368
if(!WriteFile(
366369
child_std_IN_Wr, message.c_str(), message_size, &bytes_written, NULL))
367370
{
368-
// Error handling with GetLastError ?
371+
const DWORD error_code = GetLastError();
372+
log.debug() << "Last error code is " + std::to_string(error_code)
373+
<< messaget::eom;
369374
return send_responset::FAILED;
370375
}
371376
if(bytes_written != 0)
372377
break;
373378
// Give the sub-process chance to read the waiting message(s).
379+
const auto wait_milliseconds = narrow<DWORD>(1 << send_attempts);
380+
log.debug() << "Zero bytes send to sub process. Retrying in "
381+
<< wait_milliseconds << " milliseconds." << messaget::eom;
374382
FlushFileBuffers(child_std_IN_Wr);
375383
Sleep(1 << send_attempts);
376384
}

src/util/piped_process.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ typedef struct _PROCESS_INFORMATION PROCESS_INFORMATION; // NOLINT
1313
typedef void *HANDLE; // NOLINT
1414
#endif
1515

16+
#include "message.h"
1617
#include "nodiscard.h"
1718
#include "optional.h"
1819

@@ -80,7 +81,10 @@ class piped_processt
8081
/// Initiate a new subprocess with pipes supporting communication
8182
/// between the parent (this process) and the child.
8283
/// \param commandvec The command and arguments to create the process
83-
explicit piped_processt(const std::vector<std::string> &commandvec);
84+
/// \param message_handler Optional message handler for logging debug messages
85+
explicit piped_processt(
86+
const std::vector<std::string> &commandvec,
87+
message_handlert &message_handler);
8488

8589
// Deleted due to declaring an explicit destructor and not wanting copy
8690
// constructors to be implemented.
@@ -109,6 +113,7 @@ class piped_processt
109113
int pipe_output[2];
110114
#endif
111115
statet process_state;
116+
messaget log;
112117
};
113118

114119
#endif // endifndef CPROVER_UTIL_PIPED_PROCESS_H

unit/util/piped_process.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22
/// \author Diffblue Ltd.
33
/// Unit tests for checking the piped process communication mechanism.
44

5-
# include <testing-utils/use_catch.h>
6-
# include <util/optional.h>
7-
# include <util/piped_process.h>
8-
# include <util/string_utils.h>
5+
#include <util/optional.h>
6+
#include <util/piped_process.h>
7+
#include <util/string_utils.h>
8+
9+
#include <testing-utils/message.h>
10+
#include <testing-utils/use_catch.h>
11+
912
// Used for testing destructor/timing
1013
#include <chrono>
1114

@@ -22,7 +25,7 @@ TEST_CASE(
2225
commands.push_back("/bin/echo");
2326
commands.push_back(to_be_echoed);
2427
#endif
25-
piped_processt process(commands);
28+
piped_processt process(commands, null_message_handler);
2629

2730
// This is an indirect way to detect when the pipe has something. This
2831
// could (in theory) also return when there is an error, but this unit
@@ -45,7 +48,7 @@ TEST_CASE(
4548
const std::string expected_error("Launching abcde failed");
4649
commands.push_back("abcde");
4750
#endif
48-
piped_processt process(commands);
51+
piped_processt process(commands, null_message_handler);
4952

5053
// This is an indirect way to detect when the pipe has something. This
5154
// could (in theory) also return when there is an error, but this unit
@@ -81,7 +84,7 @@ TEST_CASE(
8184
std::chrono::steady_clock::now();
8285
{
8386
// Scope restriction to cause destruction
84-
piped_processt process(commands);
87+
piped_processt process(commands, null_message_handler);
8588
}
8689
std::chrono::steady_clock::time_point end_time =
8790
std::chrono::steady_clock::now();
@@ -95,7 +98,7 @@ TEST_CASE(
9598
# if 0
9699
commands.push_back("sleep 6");
97100
time_t calc = time(NULL);
98-
piped_processt process(commands);
101+
piped_processt process(commands, null_message_handler);
99102
process.~piped_processt();
100103
calc = time(NULL) - calc;
101104
# else
@@ -114,7 +117,7 @@ TEST_CASE(
114117
std::vector<std::string> commands;
115118
commands.push_back("z3");
116119
commands.push_back("-in");
117-
piped_processt process(commands);
120+
piped_processt process(commands, null_message_handler);
118121

119122
REQUIRE(
120123
process.send("(echo \"hi\")\n") ==
@@ -136,7 +139,7 @@ TEST_CASE(
136139
commands.push_back("z3");
137140
commands.push_back("-in");
138141
const std::string termination_statement = "(exit)\n";
139-
piped_processt process(commands);
142+
piped_processt process(commands, null_message_handler);
140143

141144
REQUIRE(
142145
process.send("(echo \"hi\")\n") ==
@@ -166,7 +169,7 @@ TEST_CASE(
166169
commands.push_back("z3");
167170
commands.push_back("-in");
168171
commands.push_back("-smt2");
169-
piped_processt process(commands);
172+
piped_processt process(commands, null_message_handler);
170173

171174
std::string message =
172175
"(set-logic QF_LIA) (declare-const x Int) (declare-const y Int) (assert (> "
@@ -190,7 +193,7 @@ TEST_CASE(
190193
commands.push_back("z3");
191194
commands.push_back("-in");
192195
commands.push_back("-smt2");
193-
piped_processt process(commands);
196+
piped_processt process(commands, null_message_handler);
194197

195198
std::string statement =
196199
"(set-logic QF_LIA) (declare-const x Int) (declare-const y Int) (assert (> "
@@ -212,7 +215,7 @@ TEST_CASE(
212215
std::vector<std::string> commands;
213216
commands.push_back("z3");
214217
commands.push_back("-in");
215-
piped_processt process(commands);
218+
piped_processt process(commands, null_message_handler);
216219

217220
REQUIRE(
218221
process.send("(echo \"hi\")\n") ==
@@ -242,7 +245,7 @@ TEST_CASE(
242245
commands.push_back("z3");
243246
commands.push_back("-in");
244247
commands.push_back("-smt2");
245-
piped_processt process(commands);
248+
piped_processt process(commands, null_message_handler);
246249

247250
std::string statement =
248251
"(set-logic QF_LIA) (declare-const x Int) (declare-const y Int) (assert (> "
@@ -298,7 +301,7 @@ TEST_CASE(
298301
commands.push_back("z3");
299302
commands.push_back("-in");
300303
commands.push_back("-smt2");
301-
piped_processt process(commands);
304+
piped_processt process(commands, null_message_handler);
302305

303306
std::string statement =
304307
"(set-logic QF_LIA) (declare-const x Int) (declare-const y Int) (assert (> "

0 commit comments

Comments
 (0)