Skip to content

Commit 64152c1

Browse files
brianeggeclaude
andcommitted
Stop a long-lived process growing by the types and regexes it has read (Covered by dco/Brian_Egge.md)
Follow-up to morganstanley#549 and morganstanley#550, which bounded the fuzz harnesses against two kinds of growth and left the library's own behaviour alone. This is the library half. 1. The type memo is compacted where untrusted type descriptions are decoded. hobbes::decode interns every type it builds in the process-wide type memo (tctorMaps in lang/type.C), which holds a reference of its own to each entry and lets go only when compactMTypeMemory() is called -- which, until now, happened in exactly one place (eval/search.C). The RPC server (ipc/net.C) decodes peer-supplied type descriptions and serialized expressions on every prepare request, and the machine REPL (ipc/prepl.C) decodes the same from its driver, and neither compacted, so a server handed many distinct type descriptions grew by every one -- about 380 bytes per distinct type, for the life of the process. Compact after each request that decodes: net.C's prepare cases (0 and 1, including when they fail, since a rejected description is the one that is only garbage) and prepl.C's meta commands. What the request needed is held by the compiler and stays; what it only passed through goes. The hot paths -- invoking a prepared expression, invoking a compiled thunk -- decode nothing and are left alone. A compaction walks the memo under its lock and costs on the order of a decode, against a request that compiles an expression. unique_refc_map::get now returns its entry by value rather than by reference into the map. The reference was handed out after the lock was released, and compact() can erase that entry from another thread the moment it is -- a latent hazard that periodic compaction from a server thread would turn into a real one. The one caller (MonoType::makeType) copied the result immediately anyway. 2. A compiled regex matcher is reused for the same regexes. A regex literal is compiled into a matching function where it is read: makeRegexFn determinizes it and defines the result under a fresh name (".regex." + freshName()), and nothing removes that. morganstanley#550 measured about 83KB retained in the compiler per read of a twenty character regex. A process that reads the same regex literals repeatedly -- the same expression text arriving over RPC, a REPL session -- compiled a new matcher each time. makeRegexFn now keeps, per compiler, the matchers it has compiled, by an encoding of the regexes each one matches, and a match on the same regexes reuses the function and result mapping compiled for them: the function is pure in its input and the mapping is a property of the regexes, so neither depends on where the match that uses them is. The capture buffer expression and binding names are remade per call, because they are cheap and carry a source location. The encoding is injective by construction -- every node tagged, every name length-prefixed -- where show() is not: it prints characters raw, so 'a|b' (either) and 'a\|b' (three literals) print alike and must not share a matcher. Distinct regexes still each get a matcher; that is compiling code, and is not a leak. Measured as morganstanley#550 did, one compiler kept alive, cycles of 100 reads of the OSS-Fuzz testcase's regex with the memo compacted between cycles: before (morganstanley#550's measurement) +17.1MB per 100 reads, +8.3MB net of compaction after +0.47MB per 100 reads, +0.45MB net of compaction The remainder is the syntax-error path of the parser retaining its source text, which is documented in read/parser.C and is not touched here. The full test suite passes. test/Matching.C checks that a reused matcher matches as the first did, that the two regexes above do not share one, and that captured groups still bind through a reused matcher. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016rGT4C394qeh2DBQhqy3Tb
1 parent a340567 commit 64152c1

6 files changed

Lines changed: 137 additions & 6 deletions

File tree

include/hobbes/eval/cc.H

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <hobbes/eval/jitcc.H>
66
#include <hobbes/eval/search.H>
77
#include <hobbes/lang/expr.H>
8+
#include <hobbes/lang/pat/regex.H>
89
#include <hobbes/lang/preds/subtype/obj.H>
910
#include <hobbes/lang/tylift.H>
1011
#include <hobbes/lang/type.H>
@@ -361,6 +362,16 @@ private:
361362
bool shouldThrowOnHugeRegexDFA = false;
362363
int dfaOverNfaMaxRatio = 4;
363364

365+
// the regex matchers compiled into this compiler so far, by an encoding of
366+
// the regexes each one matches. A match on the same regexes reuses the
367+
// matcher already defined for them rather than defining another: every
368+
// regex literal read is determinized and compiled into a function here, and
369+
// nothing removes those, so this is what keeps a process that keeps reading
370+
// the same regexes -- the same expression text over RPC, say -- from
371+
// growing by a matcher per read (see makeRegexFn)
372+
friend CRegexes makeRegexFn(cc *, const Regexes &, const LexicalAnnotation &);
373+
std::unordered_map<std::string, CRegexes> regexFnCache;
374+
364375
// the bound root type-def environment
365376
using TypeAliasMap = std::map<std::string, PolyTypePtr>;
366377

include/hobbes/util/ptr.H

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,19 @@ template <typename T, typename ... Args>
1818
public:
1919
using object_type = T;
2020

21-
const std::shared_ptr<T>& get(const std::function<T*(Args...)>& mk, const Args&... args) {
21+
// returned by value rather than by reference into the map: compact() can
22+
// erase an entry from another thread as soon as the lock is released, and
23+
// a reference handed out here would then be to storage that is gone. A
24+
// copy is a reference of its own, which also keeps the entry from being
25+
// the one compact() erases while the caller still holds it.
26+
std::shared_ptr<T> get(const std::function<T*(Args...)>& mk, const Args&... args) {
2227
std::lock_guard<std::recursive_mutex> lock(mutex);
2328
auto k = std::tuple<Args...>(args...);
24-
const auto& r = this->values[k];
25-
if (r) return r;
26-
27-
this->values[k] = std::shared_ptr<T>(mk(args...));
28-
return this->values[k];
29+
auto& r = this->values[k];
30+
if (!r) {
31+
r = std::shared_ptr<T>(mk(args...));
32+
}
33+
return r;
2934
}
3035

3136
size_t compact() {

lib/hobbes/ipc/net.C

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ void evaluateNetREPLRequest(int c, void *d) {
259259
fdwrite(c, uint8_t(0));
260260
fdwrite(c, std::string(ex.what()));
261261
}
262+
compactMTypeMemory();
262263
break;
263264
case 1:
264265
// prepare a serialized expression, also return its type
@@ -286,6 +287,7 @@ void evaluateNetREPLRequest(int c, void *d) {
286287
fdwrite(c, uint8_t(0));
287288
fdwrite(c, std::string(ex.what()));
288289
}
290+
compactMTypeMemory();
289291
break;
290292
case 2:
291293
// invoke a prepared expression

lib/hobbes/ipc/prepl.C

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,16 @@ void runMachineREPLStep(cc* c) {
508508
resetMemoryPool();
509509
break;
510510
}
511+
512+
// the meta commands above decode types and expressions sent by the peer,
513+
// and every type that builds is interned in the process-wide type memo,
514+
// which holds on to it until asked not to. Ask after each one: what the
515+
// command needed is held by the compiler and stays, what it only passed
516+
// through goes. Invoking a compiled function (the default case) decodes
517+
// nothing and is the hot path, so it is left alone.
518+
if (cmd < CMD_COUNT) {
519+
compactMTypeMemory();
520+
}
511521
} catch (std::exception& ex) {
512522
std::string exn = ex.what();
513523
dbglog("*** " + exn);

lib/hobbes/lang/pat/regex.C

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,6 +1345,58 @@ ExprPtr makeRegexCaptureBuffer(const Regexes& regexes, const LexicalAnnotation&
13451345
/**************************
13461346
* make a function to determine which among the input regexes here a later string matches
13471347
**************************/
1348+
// an encoding of a regex that two regexes share only if they are the same
1349+
// regex: every node is tagged, and every name is length-prefixed, so the
1350+
// encodings of different trees cannot run together. (show() is not that: it
1351+
// prints characters raw, so 'a|b' and the three literals a, |, b look alike.)
1352+
struct regexKeyF : public switchRegex<UnitV> {
1353+
std::string* out;
1354+
explicit regexKeyF(std::string* out) : out(out) { }
1355+
1356+
UnitV with(const REps*) const override { *this->out += "e;"; return unitv; }
1357+
UnitV with(const RCharRange* x) const override {
1358+
*this->out += "r" + str::from(static_cast<int>(x->b)) + "," + str::from(static_cast<int>(x->e)) + ";";
1359+
return unitv;
1360+
}
1361+
UnitV with(const RStar* x) const override {
1362+
*this->out += "*(";
1363+
switchOf(x->v, *this);
1364+
*this->out += ")";
1365+
return unitv;
1366+
}
1367+
UnitV with(const REither* x) const override {
1368+
*this->out += "|(";
1369+
switchOf(x->lhs, *this);
1370+
*this->out += ",";
1371+
switchOf(x->rhs, *this);
1372+
*this->out += ")";
1373+
return unitv;
1374+
}
1375+
UnitV with(const RSeq* x) const override {
1376+
*this->out += ".(";
1377+
switchOf(x->lhs, *this);
1378+
*this->out += ",";
1379+
switchOf(x->rhs, *this);
1380+
*this->out += ")";
1381+
return unitv;
1382+
}
1383+
UnitV with(const RBind* x) const override {
1384+
*this->out += "b" + str::from(x->var.size()) + ":" + x->var + "(";
1385+
switchOf(x->def, *this);
1386+
*this->out += ")";
1387+
return unitv;
1388+
}
1389+
};
1390+
1391+
std::string regexFnKey(const Regexes& regexes) {
1392+
std::string k;
1393+
for (const auto& r : regexes) {
1394+
switchOf(r, regexKeyF(&k));
1395+
k += "\n";
1396+
}
1397+
return k;
1398+
}
1399+
13481400
CRegexes makeRegexFn(cc* c, const Regexes& regexes, const LexicalAnnotation& rootLA) {
13491401
CRegexes result;
13501402

@@ -1354,6 +1406,20 @@ CRegexes makeRegexFn(cc* c, const Regexes& regexes, const LexicalAnnotation& roo
13541406
result.captureVarsAt[i] = bindingNames(regexes[i]);
13551407
}
13561408

1409+
// if these regexes have been compiled into this compiler before, the
1410+
// function and the result mapping made then serve now: the function is
1411+
// pure in its input and the mapping is a property of the regexes, so
1412+
// neither depends on where the match that uses them is. (The capture
1413+
// buffer expression above is remade with this match's annotation, and the
1414+
// binding names recomputed, because they are cheap and carry a location.)
1415+
const std::string key = regexFnKey(regexes);
1416+
auto cached = c->regexFnCache.find(key);
1417+
if (cached != c->regexFnCache.end()) {
1418+
result.fname = cached->second.fname;
1419+
result.rstates = cached->second.rstates;
1420+
return result;
1421+
}
1422+
13571423
// our NFA will non-deterministically jump to every possible start state
13581424
NFA nfa;
13591425
nfa.resize(1);
@@ -1379,6 +1445,13 @@ CRegexes makeRegexFn(cc* c, const Regexes& regexes, const LexicalAnnotation& roo
13791445

13801446
// and that's the function that the outer match logic should use
13811447
result.fname = fname;
1448+
1449+
// remember the function and the result mapping for the next match on these
1450+
// regexes; not the capture buffer expression, which carries this match's
1451+
// source location and would pin it for the life of the compiler
1452+
CRegexes& kept = c->regexFnCache[key];
1453+
kept.fname = result.fname;
1454+
kept.rstates = result.rstates;
13821455
return result;
13831456
}
13841457

test/Matching.C

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,36 @@ TEST(Matching, hugeRegexDFACompilesWithoutQuadraticBlowup) {
517517
#endif
518518
}
519519

520+
// A regex literal is compiled into a matching function where it is read, and
521+
// a match on the same regexes now reuses the function compiled for them the
522+
// first time rather than defining another. Reuse has to be by the regex
523+
// itself, not by how it prints: 'a|b' (either) and 'a\|b' (three literals)
524+
// print alike but match differently, and must not share a matcher.
525+
TEST(Matching, regexMatchersAreReusedButOnlyForTheSameRegex) {
526+
auto either1 = c().compileFn<int(const std::string &)>(
527+
"s", "match s with | 'a|b' -> 1 | _ -> 0");
528+
auto either2 = c().compileFn<int(const std::string &)>(
529+
"s", "match s with | 'a|b' -> 2 | _ -> 0");
530+
auto literal = c().compileFn<int(const std::string &)>(
531+
"s", "match s with | 'a\\|b' -> 3 | _ -> 0");
532+
533+
EXPECT_EQ(either1("a"), 1);
534+
EXPECT_EQ(either1("b"), 1);
535+
EXPECT_EQ(either1("a|b"), 0);
536+
EXPECT_EQ(either2("a"), 2);
537+
EXPECT_EQ(either2("a|b"), 0);
538+
EXPECT_EQ(literal("a|b"), 3);
539+
EXPECT_EQ(literal("a"), 0);
540+
541+
// and captured groups still bind in a reused matcher
542+
auto cap1 = c().compileFn<long(const std::string &)>(
543+
"x", "match x with | '(?<pre>a+)b' -> length(pre) | _ -> 0L");
544+
auto cap2 = c().compileFn<long(const std::string &)>(
545+
"x", "match x with | '(?<pre>a+)b' -> length(pre) * 10L | _ -> 0L");
546+
EXPECT_EQ(cap1("aaab"), 3L);
547+
EXPECT_EQ(cap2("aaab"), 30L);
548+
}
549+
520550
TEST(Matching, noRaceInterpMatch) {
521551
c().alwaysLowerPrimMatchTables(true);
522552
c().buildInterpretedMatches(true);

0 commit comments

Comments
 (0)